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

Allow all constituent object detection losses to be logged #1716

Merged
merged 3 commits into from Feb 24, 2023

Conversation

AdeelH
Copy link
Collaborator

@AdeelH AdeelH commented Feb 24, 2023

Overview

This PR removes Learner.build_metric_names(). This means that all metrics returned by Learner.train_step() and Learner.validation_step() can be logged without needing to be pre-specified in Learner.build_metric_names(), which means losses from different object detection models (which can have different names) can be logged without needing to override the default ObjectDetectionLearner.

After the changes, training logs with a FasterRCNN model look like so:

{'epoch': 2,
 'loss_classifier': 0.06613687425851822,
 'loss_box_reg': 0.06521709263324738,
 'loss_objectness': 0.01976802386343479,
 'loss_rpn_box_reg': 0.0028790205251425505,
 'train_loss': 0.15400101244449615,
 'train_time': '0:00:12.718091',
 'mAP': 0.007138107431842703,
 'mAP50': 0.047601330107510685,
 'valid_time': '0:00:12.937088'}

image

With an external model (SSD):

{'epoch': 4,
 'bbox_regression': 2.0971059799194336,
 'classification': 2.8579580783843994,
 'train_loss': 4.955064296722412,
 'train_time': '0:00:19.800291',
 'mAP': 0.0027229238176515428,
 'mAP50': 0.011754015726541719,
 'valid_time': '0:00:11.715102'}

Other changes:

  • Metric aggregation has been refactored slightly.
  • New unit tests for metric aggregation and log writing.

Checklist

  • Added needs-backport label if PR is bug fix that applies to previous minor release
  • Ran scripts/format_code and committed any changes
  • Documentation updated if needed
  • PR has a name that won't get you publicly shamed for vagueness

Notes

N/A

Testing Instructions

  • Run an object detection job and check the logs.
  • See new unit tests.

@codecov
Copy link

codecov bot commented Feb 24, 2023

Codecov Report

Merging #1716 (97ad7e9) into master (8e8a20c) will increase coverage by 0.00%.
The diff coverage is 91.66%.

@@           Coverage Diff           @@
##           master    #1716   +/-   ##
=======================================
  Coverage   75.74%   75.75%           
=======================================
  Files         192      192           
  Lines        9319     9304   -15     
=======================================
- Hits         7059     7048   -11     
+ Misses       2260     2256    -4     
Impacted Files Coverage Δ
...ervision/pytorch_learner/object_detection_utils.py 77.22% <ø> (-0.13%) ⬇️
...rastervision/pytorch_learner/regression_learner.py 36.25% <0.00%> (-3.75%) ⬇️
...ch_learner/rastervision/pytorch_learner/learner.py 69.92% <90.00%> (-0.77%) ⬇️
...ervision/pytorch_learner/classification_learner.py 100.00% <100.00%> (ø)
...vision/pytorch_learner/object_detection_learner.py 92.00% <100.00%> (-0.21%) ⬇️
...pytorch_learner/object_detection_learner_config.py 84.78% <100.00%> (ø)
...n/pytorch_learner/semantic_segmentation_learner.py 98.11% <100.00%> (-0.04%) ⬇️
...earner/rastervision/pytorch_learner/utils/utils.py 94.15% <100.00%> (+0.81%) ⬆️
...er/dataset/visualizer/classification_visualizer.py 100.00% <0.00%> (+2.63%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@AdeelH AdeelH added the needs-backport This PR needs to be backported to release branches label Feb 24, 2023
@AdeelH AdeelH marked this pull request as ready for review February 24, 2023 14:08
@AdeelH AdeelH merged commit 3c45fd5 into azavea:master Feb 24, 2023
@AdeelH AdeelH deleted the metrics branch February 24, 2023 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-backport This PR needs to be backported to release branches
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant