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

Reduce predict tests #877

Merged
merged 2 commits into from
Jul 18, 2023
Merged

Reduce predict tests #877

merged 2 commits into from
Jul 18, 2023

Conversation

zhixiangteoh
Copy link
Contributor

Remove tests that test mostly already-tested code.

Fixes #876.

Remove tests that test mostly already-tested code.

Fixes #876.
@codecov
Copy link

codecov bot commented Jul 18, 2023

Codecov Report

Merging #877 (938063f) into master (8cd2ee0) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #877   +/-   ##
=======================================
  Coverage   95.65%   95.65%           
=======================================
  Files          21       21           
  Lines        2256     2256           
=======================================
  Hits         2158     2158           
  Misses         98       98           
Flag Coverage Δ
unittests 95.65% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@jeff-regier
Copy link
Contributor

jeff-regier commented Jul 18, 2023

Looks like the tests are still taking > 20 minutes, with ~12 minutes consumed test_predict_decals_multiple_bricks alone. (And another 3 minutes consumed by test_predict_sdss_multiple_rcfs.)

Could we add some (optional) configuration settings to process just subimages within bricks/fields. i.e., for each brick/field, process just on a region (perhaps an 80x80-pixel box) specified in the config? This could be useful for users too, not just for testing. Users may only be interested in a particular bounding box.

384.15s call     tests/test_predict.py::TestPredict::test_predict_decals_multiple_bricks
99.43s call     tests/test_predict.py::TestPredict::test_predict_sdss_multiple_rcfs

`predict.align` analyzed by kernprof to take ~40% runtime of predict.
Copy link
Contributor

@jeff-regier jeff-regier left a comment

Choose a reason for hiding this comment

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

Definitely an improvement, but I still hope we can get runtime below 10 minutes--see comment above.

@zhixiangteoh
Copy link
Contributor Author

Looks like the tests are still taking > 20 minutes, with ~12 minutes consumed test_predict_decals_multiple_bricks alone. (And another 3 minutes consumed by test_predict_sdss_multiple_rcfs.)

Could we add some (optional) configuration settings to process just subimages within bricks/fields. i.e., for each brick/field, process just on a region (perhaps an 80x80-pixel box) specified in the config? This could be useful for users too, not just for testing. Users may only be interested in a particular bounding box.

384.15s call     tests/test_predict.py::TestPredict::test_predict_decals_multiple_bricks
99.43s call     tests/test_predict.py::TestPredict::test_predict_sdss_multiple_rcfs

Predictions are already being done on 160x160 pixel box (testing_config.predict.crop.width|height), and profiling shows that 80x80 doesn't cut runtime much. Overriding align now which cuts a chunk of time.

Copy link
Contributor

@jeff-regier jeff-regier left a comment

Choose a reason for hiding this comment

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

Excellent! Test finish in 8 minutes with no drop in test coverage.

@jeff-regier jeff-regier merged commit 355f5db into master Jul 18, 2023
@jeff-regier jeff-regier deleted the tests/reduce-predict branch July 18, 2023 17:29
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.

pytest is slow again
2 participants