[fix/enhancement] move SN_models out of final_values#848
Open
maxbriel wants to merge 16 commits into
Open
Conversation
final_values move SN_models outfinal_values
Collaborator
Author
|
Here are the output files from the binaries that I ran with the fix and v2.3. I originally posted these on Slack and forgot to add them here: |
sgossage
reviewed
Jun 15, 2026
sgossage
reviewed
Jun 15, 2026
Comment on lines
408
to
410
| # TODO: we need to train core collapse for secondary star as well | ||
| # to catch reverse mass transfer cases where the secondary undergoes | ||
| # core collapse first |
Contributor
There was a problem hiding this comment.
We should keep this in mind as a to do item. (Not necessarily for this PR though.)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We've likely hit the maximum number of columns in a single HDF5 dataset for the
final_valueswith the increased number of SN models.As such, we need to separate out the SN_models to make sure we will not hit the same issue in the future.
I've rewritten the
add_post_processed_quantitiesto:recfuntionsfromnumpyto add all columns in one go to the structure array,final_values. Originally, we were writing one column at the time. deleting the existingfinal_valuesand then writing the newfinal_valueswith 1 extra column. Repeating this process for each new column we wanted to add (several hundred times given the number of SN_MODELS we have). This is a massive I/O overhead.SN_MODELSin their own group (grid/SN_MODELS) in the HDF5 file. Each SN_MODEL has its own dataset + in the attrs the parameters for the model are stored.grid.compression_argsare not actually populated correctly when loading a grid withgrid(path_to_file)as such when writing the "new" datasets to file, they were uncompressed.Caution
Bullet point 3 means that the
final_valuesdataset in the release dataset are uncompressed!. I verified that this is the case!Screenshots of uncompressed final_values
final_valuesinitial_valuesThis PR now also changes the
IF_interpolatorandinterpolator. If required, theIF_interpolatornow uses_get_grid_columnto get thefinal_valuesorSN_models. It only does this if it looped over the SN_MODELS before.Changes to
step_MESAis just how we access theSN_MODELdata. It should be checked if this stored everything exactly the same as before.step_SNis able to access this correctly at the moment.Warning
There is a bug I noticed with the
LazyHDF5accessing the HDF5 file when adding a column. When adding something to the HDF5f file, we reload the file, e.g.grid._reload_hdf5_file(). This invalidates theLazyHDF5linking. We should fix this, if possible, but not in this PR.