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

Add bounds checking for sncosmo models #213

Merged
merged 2 commits into from
Jan 3, 2025
Merged

Conversation

jeremykubica
Copy link
Contributor

Instead of crashing when given wavelengths out of bounds, return zero flux in those wavelengths.

@jeremykubica jeremykubica requested a review from mi-dai January 3, 2025 16:24
Copy link

github-actions bot commented Jan 3, 2025

Before [0386b25] After [4f37069] Ratio Benchmark (Parameter)
2.03±0.7ms 1.32±1ms ~0.65 benchmarks.TimeSuite.time_fnu_to_flam
16.5±0.1μs 17.1±0.2μs 1.04 benchmarks.TimeSuite.time_sample_x1_from_hostmass
5.11±0.1ms 5.24±0.2ms 1.03 benchmarks.TimeSuite.time_evaluate_salt3_passbands
1.40±0.01s 1.42±0.01s 1.01 benchmarks.TimeSuite.time_make_x1_from_hostmass
10.5±0.5ms 10.5±0.9ms 1.00 benchmarks.TimeSuite.time_evaluate_salt3_model
8.83±0.07ms 8.82±0.1ms 1.00 benchmarks.TimeSuite.time_load_passbands
28.8±0.2μs 28.9±0.4μs 1.00 benchmarks.TimeSuite.time_make_new_salt3_model
131±1μs 128±0.4μs 0.98 benchmarks.TimeSuite.time_sample_x0_from_distmod
1.52±0.1ms 1.47±0.1ms 0.96 benchmarks.TimeSuite.time_apply_passbands
5.96±0.9ms 5.45±1ms 0.91 benchmarks.TimeSuite.time_chained_evaluate

Click here to view all benchmarks.

Copy link
Contributor

@mi-dai mi-dai left a comment

Choose a reason for hiding this comment

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

This is good for now. We should allow users to fill the out-of-bound values the way they want in the future.

tests/tdastro/sources/test_sncosmo_models.py Outdated Show resolved Hide resolved
@jeremykubica jeremykubica merged commit 02815e6 into main Jan 3, 2025
5 checks passed
@jeremykubica jeremykubica deleted the sncosmo_bounds_check branch January 3, 2025 18:31
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