microsoft / nnfusion

A flexible and efficient deep neural network (DNN) compiler that generates high-performance executable from a DNN model description.
MIT License
958 stars 162 forks source link

fix: typo of scattermin op register. #414

Closed LeiWang1999 closed 2 years ago

LeiWang1999 commented 2 years ago

The original purpose of this PR is to fix the typo scattermim from issue #11

This typo will throw an operator parse exception when parse a tf.scatter_min operation, to test the scatter_min operation, I also provided a customized pb file : frozen_scattermin_graph.pb

And to test the other two registered scatter operations, I made another two pb models : frozen_scattermax_graph.pb、 frozen_scatteradd_graph.pb and assorted google test instance (All passed).

Why frozen_scattersub_graph.pb is modified?

in the original frozen_scattersub_graph.pb, the ref oprand which must be variable is value-fixed, I think it's not debug friendly and I remade it with the same skeleton of the other three pb models.

the netron's view of original and modified:

image

In this way, we can customize the value of ref for better test.

ghost commented 2 years ago

CLA assistant check
All CLA requirements met.

LeiWang1999 commented 2 years ago

Please CC @wenxcs @xysmlx

xysmlx commented 2 years ago

/azp run

azure-pipelines[bot] commented 2 years ago
Azure Pipelines successfully started running 1 pipeline(s).
wenxcs commented 2 years ago

Is there any more typo to be fixed? @LeiWang1999 @xysmlx

LeiWang1999 commented 2 years ago

@wenxcs No more in this pr, actually to detect the typo of this project I think we can use some modernization toolchains like typo inspection in clion.

image

But to do this we may need a new pr branch, for this pr, the key concept is about scatter operation test cases because this typo will throw an exception while the other won't affect the execution.

wenxcs commented 2 years ago

Great, could you help to investigate any github action that can help us do typo check on server?