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

addition of group > 1 in test and in backend #5877

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

chachaleo
Copy link

Description

I added the implementation for group > 1 in the back-end test implementation of conv transpose. And also added some tests with group > 1.

@chachaleo chachaleo requested a review from a team as a code owner January 29, 2024 08:41
Copy link

codecov bot commented Jan 29, 2024

Codecov Report

Attention: 26 lines in your changes are missing coverage. Please review.

Comparison is base (8a6b70f) 56.45% compared to head (1791b91) 56.39%.
Report is 1 commits behind head on main.

❗ Current head 1791b91 differs from pull request most recent head de7b442. Consider uploading reports for the commit de7b442 to get more accurate results

Files Patch % Lines
onnx/backend/test/case/node/convtranspose.py 0.00% 14 Missing ⚠️
onnx/reference/ops/op_conv_transpose.py 0.00% 12 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5877      +/-   ##
==========================================
- Coverage   56.45%   56.39%   -0.06%     
==========================================
  Files         504      504              
  Lines       29872    29906      +34     
  Branches     4489     4496       +7     
==========================================
+ Hits        16863    16866       +3     
- Misses      12191    12222      +31     
  Partials      818      818              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xadupre
Copy link
Contributor

xadupre commented Jan 29, 2024

You should sign off your commits (git commit -s ...) to pass DCO test (click on the link details to sign your past commits). You also to run the following commands to update the documentation and the new test in the folder onnx/backend/test/data.

python onnx/backend/test/cmd_tools.py generate-data --clean
python  onnx/defs/gen_doc.py
python onnx/backend/test/stat_coverage.py 

Signed-off-by: chachaleo <charlotte.leo@hotmail.com>
Signed-off-by: chachaleo <charlotte.leo@hotmail.com>
@xadupre
Copy link
Contributor

xadupre commented Jan 30, 2024

Is it possible to restore the original onnx models whicj already exist and keep only the one you add? That way it is easier to see your changes.

@chachaleo
Copy link
Author

Sorry yes I will clean this.

These two lines make an error :
python onnx/defs/gen_doc.py
python onnx/backend/test/stat_coverage.py

even without my changes, do I have to work on another branch than main ?

@xadupre
Copy link
Contributor

xadupre commented Jan 31, 2024

What do you mean by do I have to work on another branch than main? Submitting a PR like you did is usually what people do.

@justinchuby
Copy link
Contributor

justinchuby commented Feb 2, 2024

If I understand your question correctly, you meant if you need to create a diff against a branch other than main. Main is the correct development branch. The issue may be because a different version of protobuf / platform caused slightly different binary results for protobuf. You may include only the relavent changes in your commit by manually selecting them.

@xadupre
Copy link
Contributor

xadupre commented Feb 2, 2024

For any question related to git, some famous LLM have usually good answers to something like how to revert to the previous commit in a subdirectory using Git.

@chachaleo
Copy link
Author

Ok! :)

I actually can't manage to generate the node data, when I run

python onnx/backend/test/cmd_tools.py generate-data --clean

It only generates the node data for the existing tests, and when I run

python onnx/backend/test/cmd_tools.py generate-data --clean -d

it runs only the test of convtranspose but without the one I added.

The only changes I made was the op_convtranspose implementation and I added my test in the convtranspose.py file in
onnx>backend>test>case>node

What should I do to generate the data ?

@xadupre
Copy link
Contributor

xadupre commented Feb 5, 2024

I usually use this python onnx/backend/test/cmd_tools.py generate-data --clean to generate the data. Then I do git add ... with only the files I want to add to the PR. Then git commit -s -m .... Then I do git reset --hard to revert all the changes I do not want to include to the PR. Maybe some others have a more simple way to do it but that's what I do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

None yet

3 participants