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

Update the regression test to METAL observed/AST dataset #612

Merged
merged 6 commits into from
Sep 4, 2020

Conversation

karllark
Copy link
Member

@karllark karllark commented Sep 3, 2020

The current regression tests are based on the beast-examples/phat_small dataset (obs/AST results). The details of the phat_small dataset are lost in the mists of time. The phat_small dataset is old enough that it does not have the full information that we now use in the BEAST (e.g., the ASTs only report mags for the recovered measurements, instead of fluxes that all more recent AST results provide).

This new dataset is based on a subset of a METAL field.

A number of regression tests are skipped as they need updated cache files either for phat or metal or both. See #598.

Includes minor updates to plot_indiv_fit.py.
Includes various updates to close files left open and remove some deprecation warnings. This cleans up the test output making it easier to see problems.

Partially addresses #567.
Closes #325.

@karllark karllark marked this pull request as draft September 3, 2020 15:29
@karllark karllark added the tests label Sep 3, 2020
@codecov-commenter
Copy link

codecov-commenter commented Sep 3, 2020

Codecov Report

Merging #612 into master will decrease coverage by 4.23%.
The diff coverage is 18.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #612      +/-   ##
==========================================
- Coverage   47.25%   43.01%   -4.24%     
==========================================
  Files          94       94              
  Lines        9079     9123      +44     
==========================================
- Hits         4290     3924     -366     
- Misses       4789     5199     +410     
Impacted Files Coverage Δ
beast/plotting/plot_noisemodel.py 12.50% <ø> (ø)
beast/tools/run/create_physicsmodel.py 42.85% <ø> (-23.81%) ⬇️
beast/tools/setup_batch_beast_fit.py 8.73% <ø> (ø)
beast/observationmodel/ast/make_ast_input_list.py 7.54% <2.77%> (-1.40%) ⬇️
beast/tools/run/make_ast_inputs.py 15.51% <6.66%> (-3.24%) ⬇️
beast/physicsmodel/prior_weights_dust.py 100.00% <100.00%> (ø)
beast/plotting/plot_cmd.py 91.42% <100.00%> (+0.25%) ⬆️
beast/plotting/plot_cmd_with_fits.py 94.44% <100.00%> (+0.21%) ⬆️
beast/plotting/plot_indiv_fit.py 95.75% <100.00%> (-0.12%) ⬇️
beast/tools/convert_hdf5_to_fits.py 93.18% <100.00%> (+0.15%) ⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d26d1a...707495d. Read the comment docs.

@karllark karllark marked this pull request as ready for review September 3, 2020 20:07
Copy link
Member

@lea-hagen lea-hagen left a comment

Choose a reason for hiding this comment

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

Looks good! I like the new file organizational system for the tests (both in the new class and in the folders online).

One comment about plot_indiv_fit: the filters are still hard-coded in, so it'll only work for a particular survey. I'd imagine it would be straightforward to extract them from the stats file, and maybe use pysynphot or something to get the wavelength from the corresponding filter curve. But that may be outside the scope of this PR :).

For the tests, perhaps getting everything consistently regenerated would be a good topic for an upcoming hack day??? 😄

beast/plotting/plot_cmd_with_fits.py Show resolved Hide resolved
beast/plotting/plot_noisemodel.py Show resolved Hide resolved
beast/tests/helpers.py Outdated Show resolved Hide resolved
beast/tests/test_regresscheck.py Outdated Show resolved Hide resolved
beast/tools/tests/test_write_sbatch_file.py Show resolved Hide resolved
@karllark
Copy link
Member Author

karllark commented Sep 4, 2020

One comment about plot_indiv_fit: the filters are still hard-coded in, so it'll only work for a particular survey. I'd imagine it would be straightforward to extract them from the stats file, and maybe use pysynphot or something to get the wavelength from the corresponding filter curve. But that may be outside the scope of this PR :).

Good point! I've been wondering why plot_indiv_fit has been looking odd for the metal results. Will see what I can do about this in the next PR (not this one).

@karllark
Copy link
Member Author

karllark commented Sep 4, 2020

For the tests, perhaps getting everything consistently regenerated would be a good topic for an upcoming hack day???

Good idea. I've been thinking we should have one soon. I'll setup a doodle poll.

Copy link
Member

@lea-hagen lea-hagen left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes!

@karllark karllark merged commit 2ec0c04 into BEAST-Fitting:master Sep 4, 2020
@karllark karllark deleted the update_regtest_data branch September 9, 2020 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New observed data/ASTs for regression test
3 participants