Skip to content

[NNCF] Update test nncf_graph #1308

Merged
samet-akcay merged 5 commits intoopen-edge-platform:feature/otxfrom
AlexanderDokuchaev:ad/rework_check_nncf_graph
Nov 4, 2022
Merged

[NNCF] Update test nncf_graph #1308
samet-akcay merged 5 commits intoopen-edge-platform:feature/otxfrom
AlexanderDokuchaev:ad/rework_check_nncf_graph

Conversation

@AlexanderDokuchaev
Copy link
Copy Markdown
Contributor

@AlexanderDokuchaev AlexanderDokuchaev commented Oct 23, 2022

  • Add feature to generate reference graph by set the global variable NNCF_TEST_REGEN_DOT
  • Add reference graphs for anomaly models
  • Task: 85980, 85990, 85991. 85992

@AlexanderDokuchaev AlexanderDokuchaev requested a review from a team as a code owner October 23, 2022 11:55
@github-actions github-actions bot added API Any changes in OTX API ALGO Any changes in OTX Algo Tasks implementation labels Oct 23, 2022
@AlexanderDokuchaev AlexanderDokuchaev changed the title Update test nncf_graph [NNCF] Update test nncf_graph Oct 23, 2022
@AlexanderDokuchaev
Copy link
Copy Markdown
Contributor Author

AlexanderDokuchaev commented Oct 24, 2022

run ote-test

samet-akcay
samet-akcay reviewed Oct 27, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@samet-akcay samet-akcay left a comment

Choose a reason for hiding this comment

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

Thanks @AlexanderDokuchaev. I have just few comments regarding the PR.

I've another concern regarding the integration with the otx branch, though. We need to think about how/where we merge these changes in the feature/otx branch.

@ashwinvaidya17, maybe we could discuss this?

@AlexanderDokuchaev AlexanderDokuchaev changed the base branch from develop to feature/otx October 27, 2022 19:10
@AlexanderDokuchaev AlexanderDokuchaev force-pushed the ad/rework_check_nncf_graph branch from fb69997 to 344d9b6 Compare October 27, 2022 19:39
@AlexanderDokuchaev
Copy link
Copy Markdown
Contributor Author

AlexanderDokuchaev commented Oct 27, 2022

Changed target branch

ashwinvaidya17
ashwinvaidya17 reviewed Oct 31, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@ashwinvaidya17 ashwinvaidya17 left a comment

Choose a reason for hiding this comment

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

Thanks. I have a few comments.

ashwinvaidya17
ashwinvaidya17 reviewed Oct 31, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@ashwinvaidya17 ashwinvaidya17 left a comment

Choose a reason for hiding this comment

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

Looks good to me. I'll wait for @samet-akcay 's suggestions before approving this PR

@samet-akcay
Copy link
Copy Markdown
Contributor

samet-akcay commented Nov 1, 2022

Just to give @AlexanderDokuchaev a little background, we have been working on OTE v2, of which the first phase will soon be merged.

The tricky part in this situation is that the changes need to appear on both the develop and feature/otx branches. In the feature/otx branch, the external directory has been deprecated and will be removed in favor of the algorithms directory. In this case, task related changes should be made to algorithms/anomaly. The tests should be located in the tests directory in the root folder, not in the task implementation directory. (I'm not sure where exactly to place the tests as there is no test design for this yet.)

Do you have any ideas regarding how to merge this PR in the simplest way, @ashwinvaidya17 and @AlexanderDokuchaev?

@AlexanderDokuchaev AlexanderDokuchaev mentioned this pull request Nov 2, 2022
Merged
@AlexanderDokuchaev AlexanderDokuchaev force-pushed the ad/rework_check_nncf_graph branch from 36afaae to 78e08d1 Compare November 2, 2022 15:45
@github-actions github-actions bot added the TEST Any changes in tests label Nov 2, 2022
@AlexanderDokuchaev
Copy link
Copy Markdown
Contributor Author

AlexanderDokuchaev commented Nov 2, 2022

@samet-akcay @ashwinvaidya17
I created #1330 to develop branch.
In this PR i added changes to training_extensions/otx/* and `training_extensions/tests/*'.

samet-akcay
samet-akcay reviewed Nov 3, 2022
View reviewed changes
samet-akcay
samet-akcay reviewed Nov 3, 2022
View reviewed changes
samet-akcay
samet-akcay approved these changes Nov 3, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@samet-akcay samet-akcay left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@samet-akcay samet-akcay merged commit 4b09803 into open-edge-platform:feature/otx Nov 4, 2022
yunchu pushed a commit that referenced this pull request Nov 8, 2022
@AlexanderDokuchaev @yunchu
[NNCF] Update test nncf_graph (#1308)
57e043f 
* update check_nncf_graph

* mode get_dummy_compressed_model to common

* typing

* Update nncf_graph test in otx

* isort
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ashwinvaidya17 ashwinvaidya17 ashwinvaidya17 left review comments

@samet-akcay samet-akcay samet-akcay approved these changes

Assignees

No one assigned

Labels

ALGO Any changes in OTX Algo Tasks implementation API Any changes in OTX API TEST Any changes in tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AlexanderDokuchaev @samet-akcay @ashwinvaidya17