Skip to content

[Fix bug] IsCompatible should return false if the pass name is not found#28475

Merged
luotao1 merged 5 commits intoPaddlePaddle:developfrom
lidanqing-vv:op-version-bug
Nov 12, 2020
Merged

[Fix bug] IsCompatible should return false if the pass name is not found#28475
luotao1 merged 5 commits intoPaddlePaddle:developfrom
lidanqing-vv:op-version-bug

Conversation

@lidanqing-vv
Copy link
Copy Markdown
Contributor

@lidanqing-vv lidanqing-vv commented Nov 8, 2020

PR types

Bug fixes

PR changes

Others

Describe

if the pass name is not found but still return true, in the python fuse tests

      self.assertTrue(
          PassVersionChecker.IsCompatible("anything_pass"))

any unregistered pass name will pass UT.

@luotao1 @shangzhizhou Could you please take a look? Thanks

@paddle-bot-old
Copy link
Copy Markdown

paddle-bot-old Bot commented Nov 8, 2020

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@lidanqing-vv lidanqing-vv changed the title if the pass name is not found, IsCompatible return false if the pass name is not found, IsCompatible should return false Nov 8, 2020
@lidanqing-vv lidanqing-vv changed the title if the pass name is not found, IsCompatible should return false If the pass name is not found, IsCompatible should return false Nov 8, 2020
shangzhizhou
shangzhizhou previously approved these changes Nov 9, 2020
Copy link
Copy Markdown
Member

@shangzhizhou shangzhizhou left a comment

Choose a reason for hiding this comment

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

LGTM

@lidanqing-vv
Copy link
Copy Markdown
Contributor Author

lidanqing-vv commented Nov 10, 2020

@shangzhizhou Hi, Please review again.

  1. rename file test_conv_bias_mkldnn_fuse_pass.py to test_mkldnn_conv_bias_fuse_pass.py is necessary. Because first one will still be tested even MKLDNN is OFF but it's not what we want. After changing to test_mkldnn_conv_bias_fuse_pass.py the all CIs pass

  2. REGISTER_PASS_CAPABILITY(quant_conv2d_dequant_fuse_pass); is necessary, because in file test_trt_quant_conv2d_dequant_fuse_pass.py, PassVersionChecker.IsCompatible('quant_conv2d_dequant_fuse_pass') is called

@lidanqing-vv lidanqing-vv changed the title If the pass name is not found, IsCompatible should return false [Fix bug] If the pass name is not found, IsCompatible should return false Nov 10, 2020
Copy link
Copy Markdown
Member

@shangzhizhou shangzhizhou left a comment

Choose a reason for hiding this comment

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

LGTM

@lidanqing-vv
Copy link
Copy Markdown
Contributor Author

@luotao1 这个可以合入了吗?Intel我们组的可能不太清楚这个,然后shangzhizhou已经 approve了。

@luotao1 luotao1 merged commit 0fc181d into PaddlePaddle:develop Nov 12, 2020
@lidanqing-vv lidanqing-vv changed the title [Fix bug] If the pass name is not found, IsCompatible should return false [Fix bug] IsCompatible should return false if the pass name is not found Nov 16, 2020
@lidanqing-vv lidanqing-vv deleted the op-version-bug branch June 16, 2021 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants