[record-hessian] Introduce Record-hessian #14183
Conversation
| // Never return nullptr | ||
| HessianObserver *getObserver() const { return _observer.get(); } | ||
|
|
||
| luci::Module *_module; |
There was a problem hiding this comment.
| luci::Module *_module; | |
| luci::Module *_module = nullptr; |
| @@ -1,7 +1,7 @@ | |||
| #!/usr/bin/make -f | |||
| export DH_VERBOSE = 1 | |||
| export NNAS_BUILD_PREFIX = build | |||
| export PRESET = 20230907 | |||
| export PRESET = 20240812 | |||
There was a problem hiding this comment.
plz split infra/* to separate PRs so that compiler/* is not mixed in same PR
There was a problem hiding this comment.
Thanks for review.
Splited compiler/record-hessian with infra/*.
BLee-bot
force-pushed
the
record-hessian-2
branch
from
30fe95d
to
7e849aa
Compare
| endif(NOT ENABLE_TEST) | ||
|
|
||
| nnas_find_package(GTest REQUIRED) | ||
| set(TEST_SOURCES, "src/MinMaxComputer.cpp") |
There was a problem hiding this comment.
Q) what/where is this file?
There was a problem hiding this comment.
It should be "HessianComputer.cpp"
| if (_node->opcode() == luci::CircleOpcode::FULLY_CONNECTED || | ||
| _node->opcode() == luci::CircleOpcode::CONV_2D) | ||
| { | ||
| _hessian_computer.recordHessian(_node, tensor); |
There was a problem hiding this comment.
Q) are you going to support more opcodes ?
There was a problem hiding this comment.
Not now, but we plan to add DepthCONV or TCONV later.
| buf.swap(unfolded_buf); | ||
| } | ||
|
|
||
| void HessianComputer::recordHessian(const luci::CircleNode *node, |
There was a problem hiding this comment.
plz add dtype check inside this method
There was a problem hiding this comment.
Thank you for your suggestion. I've added a dtype check.
| std::vector<float> buf(data, data + num_elements); | ||
|
|
||
| // get the size of input channel from weights | ||
| if (node->opcode() == luci::CircleOpcode::FULLY_CONNECTED) |
There was a problem hiding this comment.
it would be better to split methods per opcode
There was a problem hiding this comment.
I've refactored the code to split the methods based on opcodes.
|
it would be better to split this PR per single purpose.
|
| @@ -0,0 +1,386 @@ | |||
| /* | |||
There was a problem hiding this comment.
these are some questions,
- what is the purpose of this file? or what does this do?
- what are the tests for the codes in this file?
There was a problem hiding this comment.
- This file records the Hessian of a model by processing input data and profiling the results. It reads input data, verifies is's type and shape, and calculates the Hessian for each.
- Tests for this file are yet to be written, but will be attached soon.
|
|
||
| void RecordHessian::initialize(luci::Module *module) | ||
| { | ||
|
|
|
CI shows several failures. plz fix them. |
| // ready to be consumed by the input circle model without any modification | ||
| // TODO reduce duplicate codes with profileRawData | ||
| std::unique_ptr<HessianMap> | ||
| RecordHessian::profileRawDataDirectory(const std::string &input_data_path) |
There was a problem hiding this comment.
this and profileRawData methods are not called from anywhere.
and also from the draft too.
plz don't put unused codes.
|
I strongly recommend you to post PRs with single context of changes that doens't exceed 100 lines with tests codes. |
|
plz read and follow https://github.com/Samsung/ONE/wiki/ONE-Developer%27s-Certificate-of-Origin |
fix filename
| endif(NOT ENABLE_TEST) | ||
|
|
||
| nnas_find_package(GTest REQUIRED) | ||
| set(TEST_SOURCES, "src/MinMaxComputer.cpp") |
There was a problem hiding this comment.
It should be "HessianComputer.cpp"
| if (_node->opcode() == luci::CircleOpcode::FULLY_CONNECTED || | ||
| _node->opcode() == luci::CircleOpcode::CONV_2D) | ||
| { | ||
| _hessian_computer.recordHessian(_node, tensor); |
There was a problem hiding this comment.
Not now, but we plan to add DepthCONV or TCONV later.
| buf.swap(unfolded_buf); | ||
| } | ||
|
|
||
| void HessianComputer::recordHessian(const luci::CircleNode *node, |
There was a problem hiding this comment.
Thank you for your suggestion. I've added a dtype check.
| std::vector<float> buf(data, data + num_elements); | ||
|
|
||
| // get the size of input channel from weights | ||
| if (node->opcode() == luci::CircleOpcode::FULLY_CONNECTED) |
There was a problem hiding this comment.
I've refactored the code to split the methods based on opcodes.
| // ready to be consumed by the input circle model without any modification | ||
| // TODO reduce duplicate codes with profileRawData | ||
| std::unique_ptr<HessianMap> | ||
| RecordHessian::profileRawDataDirectory(const std::string &input_data_path) |
| @@ -0,0 +1,386 @@ | |||
| /* | |||
There was a problem hiding this comment.
- This file records the Hessian of a model by processing input data and profiling the results. It reads input data, verifies is's type and shape, and calculates the Hessian for each.
- Tests for this file are yet to be written, but will be attached soon.
…to record-hessian-3
This PR introduces Record-hessian
Related Issue : #13480
Draft: #13585