[newchem-cpp] query inj path names#479
Open
mabruzzo wants to merge 200 commits intograckle-project:newchem-cppfrom
Open
[newchem-cpp] query inj path names#479mabruzzo wants to merge 200 commits intograckle-project:newchem-cppfrom
mabruzzo wants to merge 200 commits intograckle-project:newchem-cppfrom
Conversation
There is some unforunate consequences, but if we don't do this you end up with some pretty confusing looking code... (the confusion primarily arises if new_FrozenKeyIdxBiMap or drop_FrozenKeyIdxBiMap has different behavior from other functions with similar names). I really think it would be better to make this into a simple C++ class with a constructor and destructor, but thats a topic for another time
…rp_table_utils.hpp
There's still a lot of work to be done (the type is incomplete and isn't filled by anything at all)
We don't yet use the new format, but we are just about ready to start doing so
c35f246 to
6b69e77
Compare
Also made a few assorted fixes
Both changes essentially affected 2 constants, which prior to this commit were known as `keylen_max` & `invalid_val`. The 2 contained constants are really just implementation details that users of the inferface for `FrozenKeyIdxBiMap` shouldn't need to know anything about
…nly_load_needed_pathways
…nly_load_needed_pathways
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.
This should be reviewed after PR #478 has been reviewed and merged
The size of the PR grew a little more than I initially anticipated.
Overview
This PR exposes information about the injection-pathway data to users through injection pathway information. Specifically users can query:
"inject_model_names"to get the list of strings naming each injection pathway (in the order that is internally assumed)"inject_path_gas_yield_frac.C","inject_path_gas_yield_frac.O","inject_path_gas_yield_frac.Mg", ... to get yields for each metal nuclide that is initially in the gas-phase. For concreteness, let's consider the array associated with"inject_path_gas_yield_frac.C". Elementiof that array holds the fraction of all injected non-primordial material corresponding to Carbon nuclides in the gas phase molecules."inject_path_grain_yield_frac.MgSiO3_dust","inject_path_grain_yield_frac.AC_dust", ... to get yields for each relevant grain species. For concreteness, let's consider the array associated with"inject_path_grain_yield_frac.AC_dust". Elementiof that array holds the fraction of all injected non-primordial material initially corresponding to amorphous carbon dust.All other changes are effectively consequences
C++ Testing
I needed to update a bunch of the C++ tests to make sure that they worked properly since
"inject_model_names"is the very first queryable-key that corresponds to an array of strings.I also added a handful of tests to explicitly test queries of these new keys. Some examples include:
I did a little refactoring to try to make this all work nicely
gracklepy changes
I updated
FluidContainer(and the supporting functions) to explicitly query the list of injection pathways from Grackle, rather than hardcoding the names of all of the injection pathwaysI also added some experimental methods to access the yields from gracklepy. I don't think these are necessarily optimal, but I think its a useful short-term solution. Eventually (but not in this PR), we probably want to reuse this information to improve tests of multi-grain-species dust model so that it appropriately uses this information
pytest changes
I added a simple check of the new gracklepy changes (I think it will be a useful diagnostic tool if we make changes and forget to fully propagate the changes throughout the gracklepy)
The payoff
A major payoff to all of this effort is actually coming in the next PR.