-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
change outputs and grads from fp16-fp16-comparision to fp16-fp32 #50700
change outputs and grads from fp16-fp16-comparision to fp16-fp32 #50700
Conversation
你的PR提交成功,感谢你对开源项目的贡献! |
❌ The PR is not created using PR's template. You can refer to this Demo. |
@@ -559,6 +559,56 @@ def feed_var(self, input_vars, place): | |||
|
|||
return feed_map | |||
|
|||
# only called by the fp16 | |||
def feed_ref_var(self, input_vars, place): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个函数感觉可以跟上面的写成一个
@@ -816,6 +1073,208 @@ def _check_api_outs_by_dygraph_outs(self, api_outs, dygraph_outs, place): | |||
+ self.__class__.__name__, | |||
) | |||
|
|||
def _calc_ref_python_api_output(self, place, egr_inps=None, egr_oups=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
唯一的区别是函数中调用的append_input_output_for_dygraph
变为append_ref_input_output_for_dygraph
,可以写成一个,减少重复代码
c15ff33
to
7916071
Compare
…smile/Paddle into fp16_compare_with_paddle_fp32
@@ -1509,10 +1664,18 @@ def _compare_list(self, name, actual, expect): | |||
|
|||
def compare_single_output_with_expect(self, name, expect): | |||
actual, actual_np = self.find_actual_value(name) | |||
expect_np = expect[0] if isinstance(expect, tuple) else expect | |||
# expect, expect_np = self.find_expect_value(name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
删除多余注释代码
+ " max_tol: " | ||
+ str(max_tol) | ||
), | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这部分注释代码可删?
def _compare_numpy(self, name, actual_np, expect_np): | ||
if self.op_test.dtype == np.float16: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个判断条件是不是也需要改成is_calc_ref?
…to fp16_compare_with_paddle_fp32
…smile/Paddle into fp16_compare_with_paddle_fp32
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost LGTM。
但是有些内容不太理解和确定,请把以下内容补充到PR描述中:
- fp16测试时,构造的测试网络是什么样的,静态图最好把构造的
Program
打印出来。 - 精度阈值依照什么策略设置的。
另外,有些建议,可以下个PR完善下。
@@ -579,10 +642,10 @@ def _append_ops(self, block): | |||
else: | |||
self.infer_dtype_from_inputs_outputs(self.inputs, self.outputs) | |||
inputs = append_input_output( | |||
block, op_proto, self.inputs, True, self.dtype | |||
block, op_proto, self.inputs, True, self.dtype, self.is_calc_ref |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is_calc_ref
这个变量名从命名看,表意不太明确,建议下个PR改下名。
@@ -713,7 +782,12 @@ def create_var(np_value, name, is_input, if_return_inputs_grad_dict): | |||
lod_temp = np_value[1] | |||
|
|||
if is_input: | |||
v = self._create_var_from_numpy(np_value_temp) | |||
if is_calc_ref and np_value_temp.dtype == np.float16: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
加些注释。
stop_gradient=False, | ||
) | ||
if is_calc_ref and np_value_temp.dtype == np.float16: | ||
v = block.create_var( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不太理解,这里构造的Program
是哪样的呢?打印出来贴在PR描述里面吧。另外加些注释说明下。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
NO_FP16_COMPARED_WITH_FP32_OP_LIST = [ | ||
'fake_quantize_moving_average_abs_max', | ||
'p_norm', | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个列表建议加注释说明下,目的是什么,什么情况下可以添加op。可以下个PR修改下
PR types
New features
PR changes
Others
Describe
change outputs and grads from fp16-fp16-comparision and fp16-fp32-comparision