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

modifications to support megabeast simulations #821

Merged
merged 6 commits into from
Aug 17, 2024

Conversation

karllark
Copy link
Member

@karllark karllark commented Aug 13, 2024

The megabeast simulations are based on recomputing the prior weights using the megabeast ensemble model. This requires computing the prior weights on the full sedgrid at once. In the beast, the stellar priors are computed on the spectral grid (not sed grid) and this allows for some simplifications. Code to address computing the stellar priors on the spectral or sed grid is added. In addition, the dust priors are moved to a function so that the same function can be used in the beast and megabeast.

As part of this work, a new check is added to determine if there are repeat masses in the single age isochrone. This is the easiest way to tell if stellar prior and grid weights are being computed on the spec or sed grids (spec has no dust and is where it is done for the BEAST). Surprisingly, there is at least one case where there is a repeated mass in the spec grid - e.g., directly computed from the stellar isochrones. Thus, the new code produces slightly different results than the previous code as now the repeated masses get the correct grid weight whereas before they were not.

In addition, a bug in make_ast_input_list.py was found on line 188 when regenerating the regression test files. There must have been an upstream behavior that allowed this line to work. It logically should not have worked. @drvdputt: any insights on this? This was your code from a few years back. No worries if it doesn't spark anything. Just being comprehensive and tagging you.

Copy link

codecov bot commented Aug 16, 2024

Codecov Report

Attention: Patch coverage is 80.95238% with 8 lines in your changes missing coverage. Please review.

Project coverage is 41.97%. Comparing base (470d88e) to head (31df919).
Report is 11 commits behind head on master.

Files Patch % Lines
beast/physicsmodel/grid_and_prior_weights.py 85.71% 5 Missing ⚠️
beast/physicsmodel/creategrid.py 60.00% 2 Missing ⚠️
beast/observationmodel/ast/make_ast_input_list.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #821      +/-   ##
==========================================
+ Coverage   41.85%   41.97%   +0.11%     
==========================================
  Files         108      108              
  Lines       10281    10288       +7     
==========================================
+ Hits         4303     4318      +15     
+ Misses       5978     5970       -8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@drvdputt
Copy link

About line 188: I would expect the original to throw an exception almost every time, since idxs and rand_idx should be arrays of different sizes. So idxs == rand_idx shouldn't mean anything, and just result in a broadcasting error. But maybe the else condition that it is was just not that common, and whatever I'm doing there might just be a case of unnecessary optimization.

If you think about it, masking out at most 100k entries (the chunk size used in the algorithm) out of a grid of several million or billion SEDs, perhaps doesn't matter that much. (Although it might have mattered when I was experimenting with much larger chunk sizes).

@karllark
Copy link
Member Author

@drvdputt: thanks for the comment. This line failed when regenerating the regression test suite of files for caching. As I stated above, this PR fixes a long-standing bug we never know about. It is small, but it does change the grid weights. So, I would have thought this line would have been run many times before now. Anyway, this is why I was theorizing that maybe something upstream changed and some fall back solution is no longer valid. Regardless, going to move forward with this PR and this change as you agreed it is the correct fix.

@karllark karllark merged commit 487f14a into BEAST-Fitting:master Aug 17, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants