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

Feature: Ghost error & error pcts (#2) #624

Merged
merged 9 commits into from
May 22, 2023
Merged

Conversation

@elboy3 elboy3 requested a review from a team as a code owner May 22, 2023 16:15
@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #5478: [DQ] Add ghost errors to pred polygons.

@elboy3 elboy3 changed the base branch from main to chore/pred_polygon_dep May 22, 2023 16:27
Base automatically changed from chore/pred_polygon_dep to main May 22, 2023 16:53
@codecov-commenter
Copy link

codecov-commenter commented May 22, 2023

Codecov Report

Merging #624 (f1ff88b) into main (6900215) will decrease coverage by 0.14%.
The diff coverage is 6.66%.

@@            Coverage Diff             @@
##             main     #624      +/-   ##
==========================================
- Coverage   84.64%   84.50%   -0.14%     
==========================================
  Files         163      164       +1     
  Lines       12900    12923      +23     
==========================================
+ Hits        10919    10921       +2     
- Misses       1981     2002      +21     
Impacted Files Coverage Δ
...lity/loggers/model_logger/semantic_segmentation.py 0.00% <0.00%> (ø)
...taquality/utils/semantic_segmentation/constants.py 0.00% <0.00%> (ø)
dataquality/utils/semantic_segmentation/errors.py 0.00% <0.00%> (ø)
dataquality/schemas/semantic_segmentation.py 75.51% <100.00%> (+1.04%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@elboy3 elboy3 changed the title Feature: Ghost error (#2) Feature: Ghost error & error pcts (#2) May 22, 2023
@@ -18,6 +18,7 @@ class SemSegCols(str, Enum):
class ErrorType(str, Enum):
classification = "classification"
undetected = "undetected"
ghost = "ghost"
Copy link
Contributor

Choose a reason for hiding this comment

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

ghost or background?

Comment on lines +43 to +57
def polygon_accuracy(
preds: np.ndarray, gold_mask: np.ndarray, correct_class: int
) -> Tuple[float, Optional[int]]:
"""Calculates the accuracy of one ground truth polygon
accuracy = (number of correct pixels) / (number of pixels in polygon)

:param preds: argmax of the prediction probabilities
shape = (height, width)
:param gold_masks: ground truth masks
shape = height, width)
:param correct_class: the correct class of the polygon

returns: pixel accuracy of the predictions
"""
relevant_region = gold_mask != BACKGROUND_CLASS
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just moving this fn a little lower

Copy link
Contributor

@Ben-Epstein Ben-Epstein left a comment

Choose a reason for hiding this comment

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

approving with the context of a follow-up pr to rename ghost -> background and undetected -> missed"

Copy link
Member

@setu4993 setu4993 left a comment

Choose a reason for hiding this comment

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

Left a couple questions to improve my understanding, rest looks good.

Are there tests we can add here?

@@ -146,6 +148,7 @@ def get_polygon_data(
golds.append(polygon.label_idx)
data_error_potentials.append(polygon.data_error_potential)
errors.append(polygon.error_type.value)
error_pcts.append(polygon.error_pct)
Copy link
Member

Choose a reason for hiding this comment

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

Just to confirm: gold won't have any overlap with preds, right? Else we'll see duplicates for the same polygon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

confirmed

@@ -18,6 +18,7 @@ class SemSegCols(str, Enum):
class ErrorType(str, Enum):
classification = "classification"
undetected = "undetected"
ghost = "ghost"
Copy link
Member

Choose a reason for hiding this comment

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

Is this how these are errors are referred to in research?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. I was wondering the same - check the #cv-taskforce slack channel i just started a discussion there!

Also - in a follow up PR i am renaming this to "background" and renaming "undetected" to "missed" for consistency with OD

@elboy3
Copy link
Contributor Author

elboy3 commented May 22, 2023

Left a couple questions to improve my understanding, rest looks good.

Are there tests we can add here?

In DQ We are testing live for this sprint to get a run in sandbox 😅

It's not being used by customers and isolated code, so for now no risk.

And then dedicating time after for robust testing

@elboy3 elboy3 merged commit 504ea36 into main May 22, 2023
@elboy3 elboy3 deleted the chore/ghost_error_polygon branch May 22, 2023 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants