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

[ANOMALY] Add nncf_task #963

Merged
merged 24 commits into from
Mar 27, 2022
Merged

[ANOMALY] Add nncf_task #963

merged 24 commits into from
Mar 27, 2022

Conversation

AlexanderDokuchaev
Copy link
Contributor

@AlexanderDokuchaev AlexanderDokuchaev commented Mar 14, 2022 •
edited
Loading

@AlexanderDokuchaev AlexanderDokuchaev requested review from a team and samet-akcay March 14, 2022 04:32
@github-actions github-actions bot added DEPENDENCY Any changes in any dependencies (new dep or its version) should be produced via Change Request on PM ALGO Any changes in OTX Algo Tasks implementation labels Mar 14, 2022
@ovms-jenkins
Copy link

Can one of the admins verify this patch?

@AlexanderDokuchaev
Copy link
Contributor Author

run ote_sdk tests

samet-akcay
samet-akcay requested changes Mar 16, 2022
View reviewed changes
Copy link
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.

@AlexanderDokuchaev, thanks for the efforts for creating this huge PR! I've added my comments below

external/anomaly/ote_anomalib/tools/sample.py Outdated Show resolved Hide resolved
external/anomaly/ote_anomalib/tools/sample.py Outdated Show resolved Hide resolved
external/anomaly/ote_anomalib/tools/sample.py Outdated Show resolved Hide resolved
external/anomaly/anomaly_classification/configs/stfpm/template.yaml Show resolved Hide resolved
external/anomaly/anomaly_segmentation/configs/padim/template.yaml Show resolved Hide resolved
external/anomaly/anomaly_segmentation/configs/stfpm/template.yaml Show resolved Hide resolved
external/anomaly/ote_anomalib/nncf_task.py Outdated Show resolved Hide resolved
external/anomaly/ote_anomalib/train_task.py Show resolved Hide resolved
djdameln
djdameln requested changes Mar 22, 2022
View reviewed changes
Copy link
Contributor

@djdameln djdameln left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @AlexanderDokuchaev and sorry for the delay in reviewing it. Overall the PR looks fine, but I have a few comments.

external/anomaly/anomaly_segmentation/configs/configuration.py Outdated Show resolved Hide resolved
external/anomaly/anomaly_classification/configs/stfpm/template.yaml Show resolved Hide resolved
external/anomaly/ote_anomalib/nncf_task.py Show resolved Hide resolved
djdameln
djdameln previously approved these changes Mar 23, 2022
View reviewed changes
@AlexanderDokuchaev
Copy link
Contributor Author

run ote_sdk tests

@samet-akcay
Copy link
Contributor

@samet-akcay @djdameln i have fixed all issue from pre-commit check.
Last error in ote_sdk/ote_sdk/tests/configuration/elements/test_primitive_parameters.py, but i did not change it.

@AlexanderDokuchaev, you could perhaps remove that import if it is not used? If you recently pulled the develop branch, it might have caused such an issue.

06:41:02  flake8...................................................................Failed
06:41:02  - hook id: flake8
06:41:02  - exit code: 1
06:41:02  
06:41:02  ote_sdk/ote_sdk/tests/configuration/elements/test_primitive_parameters.py:19:1: F401 'ote_sdk.configuration.elements.utils.attr_strict_int_validator' imported but unused

@AlexanderDokuchaev
Copy link
Contributor Author

run ote_sdk tests

@AlexanderDokuchaev
Copy link
Contributor Author

AlexanderDokuchaev commented Mar 24, 2022 •
edited
Loading

Tanks @Ilya-Krylov for 4d3de88 👍

@Ilya-Krylov
Copy link
Contributor

@AlexanderDokuchaev I killed your build because it was hanging. You can try to run ./init_venv.sh locally, we experienced with such hangings if nncf is installed via git+https using latest pip

see for example how it is workarounded in mmdetection
https://github.com/openvinotoolkit/training_extensions/blob/develop/external/mmdetection/init_venv.sh#L92-L93

@AlexanderDokuchaev
Copy link
Contributor Author

@Ilya-Krylov done

@AlexanderDokuchaev
Copy link
Contributor Author

run ote_sdk tests

@Ilya-Krylov
Copy link
Contributor

@AlexanderDokuchaev something went wrong

@github-actions github-actions bot added TEST Any changes in tests API Any changes in OTX API labels Mar 25, 2022
@AlexanderDokuchaev
Copy link
Contributor Author

run ote_sdk tests

@AlexanderDokuchaev
Copy link
Contributor Author

run ote_sdk tests

@github-actions github-actions bot removed the TEST Any changes in tests label Mar 25, 2022
@AlexanderDokuchaev
Copy link
Contributor Author

run ote_sdk tests

1 similar comment
@Ilya-Krylov
Copy link
Contributor

run ote_sdk tests

@Ilya-Krylov
Copy link
Contributor

run ote_sdk tests

@Ilya-Krylov
Copy link
Contributor

run ote-test

@Ilya-Krylov
Copy link
Contributor

run ote_sdk tests

@Ilya-Krylov Ilya-Krylov merged commit e1e4fdd into develop Mar 27, 2022
@Ilya-Krylov Ilya-Krylov deleted the ad/anomaly_nncf_task branch March 27, 2022 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LeonidBeynenson LeonidBeynenson LeonidBeynenson left review comments

@Ilya-Krylov Ilya-Krylov Ilya-Krylov left review comments

@samet-akcay samet-akcay samet-akcay requested changes

@djdameln djdameln djdameln left review comments

Assignees
No one assigned
Labels
ALGO Any changes in OTX Algo Tasks implementation API Any changes in OTX API DEPENDENCY Any changes in any dependencies (new dep or its version) should be produced via Change Request on PM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@AlexanderDokuchaev @ovms-jenkins @Ilya-Krylov @samet-akcay @LeonidBeynenson @djdameln