Skip to content

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>
@chachaleo chachaleo force-pushed the convtranspose/group_n branch from 2b8d3a3 to 1791b91 Compare January 30, 2024 09:57
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
Member

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.

@xadupre
Copy link
Contributor

xadupre commented Jun 13, 2024

chachaleo, I created another PR with your changes #6175 and updated only the necessayr data, could you review it?

github-merge-queue bot pushed a commit that referenced this pull request Jun 17, 2024
### Description
Replaces #5877 as the number of impacted files is too high to be
reviewed.

---------

Signed-off-by: Xavier Dupre <xadupre@microsoft.com>
andife pushed a commit to andife/onnx that referenced this pull request Jul 20, 2024
…6175)

### Description
Replaces onnx#5877 as the number of impacted files is too high to be
reviewed.

---------

Signed-off-by: Xavier Dupre <xadupre@microsoft.com>
Signed-off-by: Andreas Fehlner <fehlner@arcor.de>
linshokaku pushed a commit to linshokaku/onnx that referenced this pull request Oct 2, 2024
…6175)

### Description
Replaces onnx#5877 as the number of impacted files is too high to be
reviewed.

---------

Signed-off-by: Xavier Dupre <xadupre@microsoft.com>
Signed-off-by: Linsho Kaku <linsho@preferred.jp>
@github-actions github-actions bot added the stale label Jun 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

3 participants