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.
TRIDENT creates ion fields by defining functions in order, creating first
ion_fraction
, thenion_number_density
,ion_density
, and finallyion_mass
. Ion fraction is arguably the main functionality of trident, looking up and interpolating a cloudy database table, with the others being more or lessyt
bookkeeping. However, this bookkeeping downplays the other dependency for ion number densities -- the number density of the underlying nuclei.By default, trident checks for metals whether there is an existing
<atom>_nuclei_mass_density
field, then a<atom>_metallicity
field, then finally a simplemetallicity
field, assuming solar abundances. Because of a bug with demeshening (#81) almost the same selection code is repeated in bothion_mass
andion_number_density
(but, this means those might not be consistent with each other, as seen in #197). For hydrogen and helium, it goes through a similar but somewhat shorter selection process.With the AGORA project, I had a unique task of comparing several different codes to each other which use different
yt
frontends, and which all have different fields available. For the sake of comparing them on as equal of footing as possible, I have been attempting several strategies to forcetrident
to use a specific "metallicity" field, instead of using their individually most optimized source, and I found this hard to do. But it's actually worse than just forcing it to usemetallicity
, because I've defined my own fieldagora_metallicity
which has been checked against each code and verified, whereas internalmetallicity
fields are sometimes incorrect or are weirdly multidimensional.Trident has no mechanism for "skipping" fields or using custom ones, and yt has no reliable way to delete the higher-priority fields. I saw @chummels has looked into field deletion before in yt-project/yt#2131, but that didn't seem to go anywhere. I personally added the
<atom>_nuclei_mass_density
fields to ART in yt-project/yt#1981, and now I am trying to intentionally neglect these fields.So, here's my solution: I added a new
metal_source
variable, to specify what fields to use as the "source" of that atom. By default, it uses the "best" source for each, and goes through the same priority logic as before, however this is now done in a new function_determine_best_source
. However, you can specify a particular priority, or if you have your own nonstandard field to access, like "agora_metallicity", pass in "custom" and give a new function to use. There are three main changes to this PR.metal_source
toadd_ion_fields
and the other functions it calls. Even though onlyadd_ion_number_density
actually uses this, since the functions call one another (though maybe this should change, see point 2), they all need access. This means that_ion_number_density
is now a nested function inside a generator, which will decide for itself on the "source" to use if "best" is passed in. Also, added docstrings explaining this, and logging to the user.add_ion_fields
to explicitly call all four field functions, instead of calling one, which calls another, which calls another, etc. This just makes the code more readable. Instead ofadd_ion_mass
always callingadd_ion_density
, it just checks if the ion density field exists, and calls it if it doesn't.(However, in my opinion it would be a good idea to consider removing this functionality entirely, and requiring users to use the preferred method
add_ion_fields
instead of using these individual functions. This way, each field can depend on the prior ones but doesn't have to keep all the info to create them. But, this change would require lots of advance warning in case users rely on those functions)_ion_mass
to use_ion_density
as its source, instead of stepping through the priority list again. In my experience, thiseffective_volume
trick works equally well for SPH and AMR codes, though any info people have on this would be appreciated.