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 bug of fp16 converter about the cast node and topology in sub-graph #291

Merged
merged 19 commits into from
Jun 6, 2024

Conversation

xiaowuhu
Copy link
Collaborator

@xiaowuhu xiaowuhu commented May 24, 2024

There have a lot of bugs in the float16 converter after ORT 1.17 released, because the optimization rule changed in ORT. And customers also raised a lot of fp16 converter issue, which to be considered as corner case, but also need to fix. The original implementation is hard to understand, less maintainable. So I rewrite the function convert_float_to_float16, the logic is simple to understand, and easy to modify each components.
Also added 3 new features:

  • remove identity node when it is unnecessary.
  • remove cast node when it is unnecessary.
  • sort topology if ORT complain the topology is not in order, the input of node 'x' is not output of any previous node.

@xiaowuhu xiaowuhu marked this pull request as draft May 24, 2024 06:05
@xiaowuhu xiaowuhu marked this pull request as ready for review June 5, 2024 11:16
@xiaowuhu xiaowuhu requested a review from fatcat-z June 5, 2024 11:16
@fatcat-z
Copy link

fatcat-z commented Jun 5, 2024

Please add some descriptions so we know what kind of issue this PR aims to address.

tests/test_float16.py Outdated Show resolved Hide resolved
@xiaowuhu xiaowuhu requested a review from fatcat-z June 6, 2024 06:50
@xiaowuhu xiaowuhu requested a review from fatcat-z June 6, 2024 08:10
@xiaowuhu xiaowuhu changed the title Xiaowu/fixfp16oldbug Fix bug of fp16 converter about the case node and topology in sub-graph Jun 6, 2024
@xiaowuhu xiaowuhu changed the title Fix bug of fp16 converter about the case node and topology in sub-graph Fix bug of fp16 converter about the cast node and topology in sub-graph Jun 6, 2024
Copy link

@fatcat-z fatcat-z left a comment

Choose a reason for hiding this comment

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

Thanks!

@xiaowuhu xiaowuhu merged commit 8beb9a2 into master Jun 6, 2024
9 checks passed
@xiaowuhu xiaowuhu deleted the xiaowu/fixfp16oldbug branch June 6, 2024 09:32
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