add fake_quantization op conversion in onnx #4512
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
daquexian
reviewed
Mar 27, 2021
|
|
||
|
|
||
| def set_moving_max_min_value(): | ||
| max_key, min_key = "", "" |
There was a problem hiding this comment.
这里初始值设成 None（因为 None 一定是一个不合法的值），在 31 行检查两个 key 都不是 None
| node.output_tensor_names[0], | ||
| name=id_util.UniqueStr(node.name), | ||
| ) | ||
| if opset == 10: |
There was a problem hiding this comment.
这里改成 opset < 13，173 行改成 else
|
|
||
| def test_fake_quantization_affine_gpu_moving_average(test_case): | ||
| generate_fake_quantization_test_moving_average( | ||
| formula="google", scheme="affine", device_type="gpu" |
There was a problem hiding this comment.
formula 只能是 google，这个参数可以去掉。
然后可以参考一下其它单测里，用 GenArgDict 自动生成参数组合的方式
daquexian
reviewed
Mar 27, 2021
| @@ -123,6 +124,7 @@ std::string OpTypeName4OpNode(const OpNode* node) { | |||
|
|
|||
| using OpConfMap = HashMap<std::string, OperatorConf>; | |||
|
|
|||
| template<DataType VALUE_TYPE = DataType::kFloat> | |||
There was a problem hiding this comment.
这里不需要大写，就叫 data_type 就可以
| return GenLogicalBlobName(train_step_var.name(), | ||
| train_step_var.variable_conf().out()); | ||
| } | ||
| }()) |
There was a problem hiding this comment.
这里写成普通的顺序代码吧，现在这样定义 lambda 并立即调用的易读性不高，如果把这个 lambda 函数取个名字再调用的话，一是这段代码只会执行一遍，二是这段代码有副作用（创建 variable op），本身就不能多次执行。所以直接改成顺序代码吧
daquexian
reviewed
Mar 27, 2021
| @@ -255,22 +255,19 @@ user_op::UserOpConfWrapper MovingMinMaxObserver(const std::string& name, const s | |||
| Get1DZeroVariableOpConf(moving_max_name, scope_symbol_id, 1, inserted_ops); | |||
| const auto moving_min_var = | |||
| Get1DZeroVariableOpConf(moving_min_name, scope_symbol_id, 1, inserted_ops); | |||
| std::string train_step_value = train_step_lbn; | |||
There was a problem hiding this comment.
这里换一个清晰一些的名字，例如 observer_current_train_step
daquexian
approved these changes
Mar 27, 2021
mosout
requested review from
oneflow-ci-bot
and removed request for
oneflow-ci-bot
mosout
requested review from
oneflow-ci-bot
and removed request for
oneflow-ci-bot
daquexian
requested review from
oneflow-ci-bot
and removed request for
oneflow-ci-bot
liujuncheng
pushed a commit
that referenced
this pull request
Jun 3, 2021
* add min_max_observer and moving_average_min_max_observer conversion in onnx * fix op's name * fix moving_average_min_max_observer * update quantization ops conversion * update quantization ops conversion and tests * update ops version * delete auto-imported package * update min_max_observer op * format * fix test_quantization_aware_training * update test_quantize_op * add fake_quantization conversion in onnx * format quantize.py * fix fake_quantization * update fake_quantization conversion * update fake_quantization conversion and its test * update quantization_aware_training * update quantization_aware_training Co-authored-by: oneflow-ci-bot <69100618+oneflow-ci-bot@users.noreply.github.com> Former-commit-id: 1fef03a
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.