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

Convert adaptive_threshold to Enum in configs #637

Merged
merged 23 commits into from
Oct 18, 2022
Merged

Convert adaptive_threshold to Enum in configs #637

merged 23 commits into from
Oct 18, 2022

Conversation

samet-akcay
Copy link
Contributor

Description

Currently the threshold settings are as follows:

metrics:
  image:
    - F1Score
    - AUROC
  pixel:
    - F1Score
    - AUROC
  threshold:
    image_default: 3
    pixel_default: 3
    adaptive: true

This PR refactors this and converts adaptive to threshold.method using ThresholdMethod enum. In addition, it also creates NormalizationMethod enum.

Changes

  • Bug fix (non-breaking change which fixes an issue)
  • Refactor (non-breaking change which refactors the code base)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist

  • My code follows the pre-commit style and check guidelines of this project.
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes

@samet-akcay
Rename image/pixel_metrics_names to image/pixel_metrics
80f47b4
@samet-akcay
Rename image/pixel_metrics_names to image/pixel_metrics
8039413
@samet-akcay
Modified config files.
2e6b831
@samet-akcay
Modify get_callbacks function for the old cli
8dce79e
@samet-akcay
Create pre-processing-configuration callback
508d469
@samet-akcay
Add new CLI configuration for the post-processing configuration
e1a0671
@samet-akcay
Add options to normalization_method
b9ae696
@samet-akcay
Address mypy issues
00202c8
@samet-akcay
Fix docstring
54c9a39
@samet-akcay
Fix merge conflicts
ba450f4
@samet-akcay
Address codacy issues
838bd0b
@samet-akcay
Merge branch 'main' of github.com:openvinotoolkit/anomalib into cli/s…
567bb9b 
…a/create-post-processing-section-in-configs
@samet-akcay
renamed adaptive: true to threshold_method: adaptive in config.yaml f…
61352ae 
…iles
@samet-akcay
Reorder threshold params in config.yaml
6ce5f71
@samet-akcay
Add backward compatibility to threshold configs
c234967
@samet-akcay
renamed the new cli config files
86156d1
@samet-akcay
Rename threshold_method to method in config.yaml
b2426e8
@samet-akcay
Convert names to Enum
2756b8d
@github-actions github-actions bot added Callbacks CLI Config HPO Post-Processing The components that are related to post-processing Tests labels Oct 18, 2022
@github-actions github-actions bot added the Metrics Metric Component. label Oct 18, 2022
djdameln
djdameln reviewed Oct 18, 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! good addition. One minor comment/question

anomalib/post_processing/post_process.py Outdated
"""Threshold method to apply post-processing to the output predictions."""

ADAPTIVE = "adaptive"
FIXED = "fixed"
Copy link
Contributor

@djdameln djdameln Oct 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Do you think maybe manual would be a better name instead of fixed? As in manual threshold vs. adaptive threshold. (Sorry for being annoying, I realize I was the one who proposed fixed initially).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm fine with manual. @ashwinvaidya17, do you have any preference?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Works for me too

ashwinvaidya17
ashwinvaidya17 approved these changes Oct 18, 2022
View reviewed changes
Copy link
Collaborator

@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!

djdameln
djdameln approved these changes Oct 18, 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!

@samet-akcay samet-akcay merged commit dacf3f4 into main Oct 18, 2022
@samet-akcay samet-akcay deleted the refactor/sa/convert_adaptive_threshold_to_enum branch October 18, 2022 14:17
@samet-akcay samet-akcay mentioned this pull request Oct 20, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashwinvaidya17 ashwinvaidya17 ashwinvaidya17 approved these changes

@djdameln djdameln djdameln approved these changes

Assignees
No one assigned
Labels
CLI Config HPO Metrics Metric Component. Post-Processing The components that are related to post-processing Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Convert adaptive_threshold to Enum in configs
3 participants
@samet-akcay @ashwinvaidya17 @djdameln