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

[Torch] Fix classification sample metric dumps not in main process #2146

Merged

Conversation

daniil-lyakhov
Copy link
Collaborator

@daniil-lyakhov daniil-lyakhov commented Sep 18, 2023

Changes

In torch classification sample: write_metrics and mlflow accuracy logging is called only in main process

Reason for changes

Torch test_compression_training (build 106) failed due to metric collected by write_metrics and metric saved in checkpoint with key "best_acc1" were different

Tests

Torch test_compression_training build 109 111

@daniil-lyakhov daniil-lyakhov requested a review from a team as a code owner September 18, 2023 16:48
@daniil-lyakhov daniil-lyakhov changed the title Fix classification sample metric dumps not in main process [Torch] Fix classification sample metric dumps not in main process Sep 18, 2023
@github-actions github-actions bot added the NNCF PT Pull requests that updates NNCF PyTorch label Sep 18, 2023
@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Merging #2146 (c67e51c) into develop (8ceff78) will increase coverage by 0.01%.
Report is 1 commits behind head on develop.
The diff coverage is n/a.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2146      +/-   ##
===========================================
+ Coverage    36.23%   36.24%   +0.01%     
===========================================
  Files          477      477              
  Lines        42544    42570      +26     
===========================================
+ Hits         15414    15428      +14     
- Misses       27130    27142      +12     

see 6 files with indirect coverage changes

Copy link
Contributor

@vshampor vshampor left a comment

Choose a reason for hiding this comment

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

Mainly OK, one change to be addressed

Comment on lines 731 to 732
if is_main_process():
if log_validation_info:
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep the original version with and to have less levels of indent

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@daniil-lyakhov
Copy link
Collaborator Author

Build is successful, it's safe to merge

@ljaljushkin ljaljushkin merged commit 0af613d into openvinotoolkit:develop Sep 21, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NNCF PT Pull requests that updates NNCF PyTorch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants