Skip to content
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

Fix upsample shape infer bug continue #8159

Merged

Conversation

zhongshsh
Copy link
Contributor

@zhongshsh zhongshsh commented May 7, 2022

fix upsample shape infer bug caused by float precision. linked:

@@ -419,7 +419,7 @@ def test_upsample1d_nearest_output_size(test_case):
m = torch.nn.Upsample(size=(13), mode="nearest")
y = m(x)
return y

@autotest(n=5, atol=1e-5)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

除了linear之外,需要把其它的测试都限制在只测cuda,因为pytorch cpu版本的算子有点bug。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

收到

@@ -9118,6 +9119,7 @@ def OneFlow_UpsampleBicubic2DGradOp : OneFlow_BaseOp<"upsample_bicubic_2d_grad",
DefaultValuedAttr<F32Attr, "0.">:$height_scale,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

还有一些细节需要处理一下,比如:

Suggested change
DefaultValuedAttr<F32Attr, "0.">:$height_scale,
DefaultValuedAttr<F64Attr, "0.">:$height_scale,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已改

const bool& align_corners, const std::string& data_format) const {
const bool& align_corners,
const Optional<std::vector<int64_t>>& output_size,
const std::string& data_format) const {
MutableAttrMap attrs;
JUST(attrs.SetAttr<float>("depth_scale", depth_scale));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
JUST(attrs.SetAttr<float>("depth_scale", depth_scale));
JUST(attrs.SetAttr<double>("depth_scale", depth_scale));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已改

@@ -22,8 +22,8 @@ namespace one {

struct UpsampleCaptureState : public AutoGradCaptureState {
bool requires_grad;
float height_scale;
float width_scale;
double height_scale;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

给默认值

@zhongshsh zhongshsh merged commit feed1bb into fix_upsample_shape_infer_bug May 11, 2022
@zhongshsh zhongshsh deleted the fix_upsample_shape_infer_bug_continue branch May 11, 2022 06:07
BBuf added a commit that referenced this pull request May 21, 2022
* fix upsample shape infer bug

* add more example

* fix eager free tensor bug when in job

* fix nearest2d bug

* fix upsample nearest1d shape infer bug

* restruct upsample op

* change all float scale to double

* Fix upsample shape infer bug continue (#8159)

* fix_upsample_shape_infer_bug

* fix 5 nearest

* add 5 nearest test

* fix 5 nearest test

* fix 1 linear

* fix 4 bilinear

* fix 4 bicubic

* modify bicubic 2d file name

* fix 5 trilinear

* fix exception info

* fix exception info

* fix bug

* modify interpolate

* change float to double

* rm useless SI64ArrayAttr: in OneFlowUserOps

* rm useless import in cpp

* update

* add judge for output_size

* update oneflow/oneflow/core/autograd/gradient_funcs/upsample.cpp

* add grad in td

* test failed

* fix small failed case in upsample

* solve test error

* change float to double

* align to fix_upsample_shape_infer_bug

* align to fix_upsample_shape_infer_bug

Co-authored-by: BBuf <1182563586@qq.com>

* fix comment

* fix test bug

* fix comment

* fix jiebao commnet

* fix comment

* fix commnet

* all scale to double

* auto format by CI

* fix clang tidy bug

* fix clang tidy warning

* relax speed test

* simplify test case

* fix bug

Co-authored-by: Shanshan Zhong <62104945+zhongshsh@users.noreply.github.com>
Co-authored-by: ZZK <42901638+MARD1NO@users.noreply.github.com>
Co-authored-by: oneflow-ci-bot <ci-bot@oneflow.org>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants