Always enable time-dependent maps

[nilsvu]

Proposed changes

This enables time-dependent maps at runtime in the binary domains also for elliptic executables (the other domains already enable time-dependent maps at runtime). We'll use this for deformations of the domain that can be updated throughout the solve.

Upgrade instructions

Code review checklist

• The code is documented and the documentation renders correctly. Run
  make doc to generate the documentation locally into BUILD_DIR/docs/html.
  Then open index.html.
• The code follows the stylistic and code quality guidelines listed in the
  code review guide.
• The PR lists upgrade instructions and is labeled bugfix or
  new feature if appropriate.

Further comments

[nilsvu]

@nikwit could you check if this change is acceptable for you? (since you added this line)

[nikwit]

This will work but slow things down for non-moving meshes. Is this change just for cleaning up code?

[knelli2]

Could you edit the action to check if the domain is time dependent, and if it isn't just return?

[nikwit]

The problem there is that we use the same action CalculateGrVars in the initialization_actions where it is always needed and in the step_actions where it is only needed if the domain is time dependent. This can be fixed by e.g. checking if the tags are initialized or simply splitting into two actions, InitializeGrVars and UpdateGrVars.

This actually makes me realize that the current system will not work with AMR as any change in the coordinate locations would need to re-trigger CalculateGrVars, even for time-independent domains. If the background tags were a compute tag that depends on the inertial coordinates, that would be robust to handle both time dependence and AMR. However, this would re-compute the tags also for a time independent domain whenever the time changes, as tags cannot indicate at the moment whether a mutate has actually changed them. We discussed a way to implement this about two years ago but abandoned it as far as I remember. Unless someone has a better idea, I think it would be safest to convert it into a compute tag at this point and take the performance hit for time-independent maps.

For now I think the PR can be merged; at worst it will bring a performance penalty.

[nilsvu]

Ok thanks for checking. I'll check if it's easy to skip the computation for time-independent blocks, or else add a comment on the performance implications. Note that the compile-time flag was only used for the binary domain; the others decide at runtime and at block-level whether or not they are time dependent, so the runtime check is better anyway.

AMR is not really a problem here. For AMR you will add projectors that handle each tag in the DataBox when the grid changes. The projector will just re-evaluate the GR background. No need to make this a compute tag for that.

[nikwit]

I can do that if you like. In that case its probably best to add a template <bool Initialize> to CalculateGrVars. If true, always evaluate, else only evaluate if domain.is_time_dependent(). Then CalculateGrVars<true> is added to the initialization actions and called by AMR and CalculateGrVars<false> is in the step actions.

[nilsvu]

Done in a fixup. Please take a look.

[nikwit]

The changes look like what I had in mind. The test of CalculateGrVars needs to be updated though and it looks like you are missing an include for the domain tag. You can squash this in.

[nilsvu]

Done

[knelli2]

One thing. You can choose whether you believe it warrants a change. Also maybe @nikwit should weigh in because he'll be the one using it