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

Fix OD benchmarks and disallow calculating DetailedPRCurves when using AnnotationType.RASTER #726

Merged
merged 8 commits into from
Aug 28, 2024

Conversation

ntlind
Copy link
Contributor

@ntlind ntlind commented Aug 28, 2024

Improvements

  • Fix a bug in the benchmarking script where we weren't actually telling valor_core to calculate DetailedPrecisionRecallCurve
  • Throw a NotImplementedError when a user tries to calculate DetailedPrecisionRecallCurve using AnnotationType.RASTER.
    • Currently, valor_core will try to produce (datum_uid, mask) pairs, which easily leads to OOM issues since we're creating sets of tuples, each containing relatively large masks
    • These curves were never intended to be used with rasters (just bounding boxes and polygons), so I'm recommending we just throw a NotImplementedError for now. when we re-prioritize DetailedPRCurves in the future, we can discuss what the right output is for this metric.
  • Disable compute_detailed for the Box and Polygon OD benchmarks since a) they make the tests take a while to run (~80 seconds for just one run of 5000 Box datums; 213 seconds for one Polygon run) and b) profiling and improving these PR curves is low priority right now. We can try adding them back after Justin merges his PRs.
  • Update the valor_core OD benchmarks to use the ValorDetectionManager class
  • Refactor the raster IOU function to make it easier to read

@ntlind ntlind requested review from czaloom and ekorman as code owners August 28, 2024 16:48
@ntlind ntlind self-assigned this Aug 28, 2024
@ntlind ntlind changed the title Disallow calculating DetailedPRCurves when using AnnotationType.RASTER Fix OD benchmarks and disallow calculating DetailedPRCurves when using AnnotationType.RASTER Aug 28, 2024
@ntlind ntlind merged commit cbd1051 into main Aug 28, 2024
14 checks passed
@ntlind ntlind deleted the disallow_raster_detaild_pr_curves branch August 28, 2024 21:20
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.

2 participants