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.

Sign up for GitHub

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

Jump to bottom

[TorchFX] Graph tests extended with edge shapes #2918

Merged
merged 1 commit into from
Sep 30, 2024
Merged

[TorchFX] Graph tests extended with edge shapes #2918

merged 1 commit into from
Sep 30, 2024

Conversation

daniil-lyakhov
Copy link
Collaborator

@daniil-lyakhov daniil-lyakhov commented Aug 27, 2024 •
edited
Loading

Changes

Graph tests extended with edges shape

Reason for changes

To test edge shapes by graph tests

Related tickets

#2904

@daniil-lyakhov daniil-lyakhov changed the title [TorchFX] Graph tests extended with edge shapes WIP [TorchFX] Graph tests extended with edge shapes Aug 27, 2024
@github-actions github-actions bot added NNCF PT Pull requests that updates NNCF PyTorch experimental NNCF OpenVINO Pull requests that updates NNCF OpenVINO NNCF ONNX Pull requests that updates NNCF ONNX NNCF PTQ Pull requests that updates NNCF PTQ labels Aug 27, 2024
@daniil-lyakhov daniil-lyakhov force-pushed the dl/fx/model_transformer_tests branch from 17887eb to 2925666 Compare August 27, 2024 12:17
@daniil-lyakhov daniil-lyakhov force-pushed the dl/fx/model_transformer_tests branch from 2925666 to 6b4d29d Compare September 19, 2024 09:55
@github-actions github-actions bot removed experimental NNCF OpenVINO Pull requests that updates NNCF OpenVINO NNCF ONNX Pull requests that updates NNCF ONNX NNCF PTQ Pull requests that updates NNCF PTQ labels Sep 19, 2024
@daniil-lyakhov daniil-lyakhov force-pushed the dl/fx/model_transformer_tests branch 3 times, most recently from 16c164c to 44746bc Compare September 19, 2024 11:44
@daniil-lyakhov daniil-lyakhov marked this pull request as ready for review September 19, 2024 11:57
@daniil-lyakhov daniil-lyakhov requested a review from a team as a code owner September 19, 2024 11:57
@daniil-lyakhov daniil-lyakhov mentioned this pull request Sep 19, 2024
Merged
@daniil-lyakhov daniil-lyakhov force-pushed the dl/fx/model_transformer_tests branch 2 times, most recently from 388fdca to ef526ac Compare September 20, 2024 13:03
@daniil-lyakhov daniil-lyakhov changed the title WIP [TorchFX] Graph tests extended with edge shapes [TorchFX] Graph tests extended with edge shapes Sep 20, 2024
@daniil-lyakhov daniil-lyakhov mentioned this pull request Sep 20, 2024
Merged
AlexanderDokuchaev
AlexanderDokuchaev requested changes Sep 23, 2024
View reviewed changes
.../fx/reference_graphs/extracted_graphs/ConvolutionWithAllConstantInputsModelconv2d_conv2d.dot Outdated Show resolved Hide resolved
tests/torch/fx/test_models.py Outdated
FX_DIR_NAME = Path("fx")
FX_QUANTIZED_DIR_NAME = Path("fx") / "quantized"

def check_fx_graphs(graph: NNCFGraph, path_to_dot: str, graph_dir: str):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is not fx specific change, please add new arg extended to existed function

Copy link
Collaborator Author

@daniil-lyakhov daniil-lyakhov Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Then all references graphs will be placed back to directory "tests/torch/data/reference_graphs/fx" and other references like metatypes will be placed to "tests/torch/data/fx/reference_metatypes", are you ok with that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This behavior changes in this PR? Please write about all changes in PR description.

But i suggest firstly discuss about changes in structure of tests on meeting.

Why do you prefer tests/torch/data/fx but not tests/torch/fx/data?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have no preferences, please tell me the right structure and I'll do it

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you have no preferences about it, lets keep structure as it.
And will discuss about structure on meeting.

AlexanderDokuchaev
AlexanderDokuchaev requested changes Sep 23, 2024
View reviewed changes
tests/torch/fx/test_models.py Outdated
FX_DIR_NAME = Path("fx")
FX_QUANTIZED_DIR_NAME = Path("fx") / "quantized"

def check_fx_graphs(graph: NNCFGraph, path_to_dot: str, graph_dir: str):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This behavior changes in this PR? Please write about all changes in PR description.

But i suggest firstly discuss about changes in structure of tests on meeting.

Why do you prefer tests/torch/data/fx but not tests/torch/fx/data?

@daniil-lyakhov daniil-lyakhov force-pushed the dl/fx/model_transformer_tests branch from ef526ac to abc8e95 Compare September 27, 2024 09:17
@daniil-lyakhov
Copy link
Collaborator Author

@AlexanderDokuchaev, I keep the ref structure as is is, please merge if you have no comments

@AlexanderDokuchaev AlexanderDokuchaev merged commit 2c8b70c into openvinotoolkit:develop Sep 30, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexanderDokuchaev AlexanderDokuchaev AlexanderDokuchaev approved these changes

Assignees
No one assigned
Labels
NNCF PT Pull requests that updates NNCF PyTorch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@daniil-lyakhov @AlexanderDokuchaev