coveralls
commented
1 year ago Pull Request Test Coverage Report for Build 8f10cd15-bfa7-48a4-8ac0-e17d6455a69b
- 0 of 0 changed or added relevant lines in 0 files are covered.
- 40 unchanged lines in 1 file lost coverage.
- Overall coverage decreased (-0.09%) to 75.577%
| Files with Coverage Reduction | New Missed Lines | % | ||
|---|---|---|---|---|
| /home/circleci/trident/trident/ion_balance.py | 40 | 82.88% | ||
| <!-- | Total: | 40 | --> |
| Totals | |
|---|---|
| Change from base Build b3b725de-e97a-4358-8225-f0648498ecac: | -0.09% |
| Covered Lines: | 1767 |
| Relevant Lines: | 2338 |
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 lessytbookkeeping. 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_densityfield, then a<atom>_metallicityfield, then finally a simplemetallicityfield, assuming solar abundances. Because of a bug with demeshening (#81) almost the same selection code is repeated in bothion_massandion_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
ytfrontends, 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 forcetridentto 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_metallicitywhich has been checked against each code and verified, whereas internalmetallicityfields 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_densityfields 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_sourcevariable, 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_sourcetoadd_ion_fieldsand the other functions it calls. Even though onlyadd_ion_number_densityactually 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_densityis 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_fieldsto 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_massalways 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 methodadd_ion_fieldsinstead 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_massto use_ion_densityas its source, instead of stepping through the priority list again. In my experience, thiseffective_volumetrick works equally well for SPH and AMR codes, though any info people have on this would be appreciated.