Fix gizmo frontend #3234
Merged
Fix gizmo frontend #3234
Conversation
This file contains 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
This change doesn't look like it preserves backwards compatibility. Does it ? |
neutrinoceros
approved these changes
Apr 29, 2021
brittonsmith
approved these changes
Apr 29, 2021
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.
Two small changes to fix the gizmo frontend.
One, gizmo is still expecting the
H_number_densityfield to exist to calculate the free electron content. RecentlyH_number_densitygot swapped toH_nuclei_density. This corrects that.Another issue is that increasingly the FIRE datasets are using variable numbers of metallicity fields above 11 elements. When the gizmo frontend was first created, it was set to accept 11 and 17 metallicity fields to indicate it was a FIRE (i.e., gizmo) dataset to differentiate it from Gadget datasets. This has been somewhat relaxed and there are a number of FIRE datasets with 12, 13, etc. metallicity fields floating around. This fixes this so any gadget-like dataset with more than 11 metallicity fields gets flagged as a gizmo dataset.
In the long run, it seems like it might be more appropriate to change the gizmo frontend to be the FIRE frontend, since gizmo is meant to just use the same format as gadget and have the same modularity to have many different code_units set. Currently the gizmo frontend is really acting to set the code units according to the FIRE defaults. After all, we have an EAGLE and an OWL frontend, which are just extensions of the Gadget frontend for those datasets. Furthermore, Phil has updated gizmo so the newest datasets have HDF attributes to describe the code_unit conversion to physical units, but those aren't widespread yet. The correction in this PR is just a stop-gap.