Allow reference tags to list multiple arguments #3142
Conversation
|
@kidder The unit tests pass, but could you make sure I didn't majorly mess up the compute-item dependency tracking? I'll also add more tests. |
There was a problem hiding this comment.
Looks good. I would like to see this feature merged :)
Might be good to add a reference tag that depends on a reference tag? Something like if some bool is true then choose tag Scalar1 otherwise Scalar2 where Scalar1 and Scalar2 are in two different Variables. Then test that mutating the bool, or either of the Variables tags has the tag be updated. I think this will test the new functionality. You could do something similar with compute tags + subitems to be extra sure since probably any bug in this will be horrible to track down in an executable.
nilsvu
force-pushed
the
reftags_args
branch
2 times, most recently
from
633f898
to
ff39be7
Compare
|
@kidder I have rebased this PR, simplified it, and added some more tests. Could you take a look please? |
src/DataStructures/DataBox/Tag.hpp
Outdated
| * a const reference to the sub-item | ||
| * - type alias `argument_tags` that is `tmpl::list<parent_tag>` | ||
| * - type alias `argument_tags` that lists tags needed to evaluate `get` | ||
| * - static function `get` that, given the item fetched by `argument_tags`, |
This actually simplifies the DataBox logic because reference tag dependencies are treated the same as compute item dependencies.
Proposed changes
Remove
parent_tags and useargument_tags instead. This removes the restriction that reference tags can only have a single argument. My use case for this is to store an elastic constitutive relation per block in the global cache and reference the correct one in the element's DataBox, which needs both the global cache tag and the element ID.Upgrade instructions
Code review checklist
make docto generate the documentation locally intoBUILD_DIR/docs/html.Then open
index.html.code review guide.
bugfixormajor new featureif appropriate.Further comments