Skip to content

Calculate yup/ydown on demand - #1342

Closed
johnomotani wants to merge 7 commits into
shiftedmetric-updatesfrom
calcYupYdown-on-demand
Closed

Calculate yup/ydown on demand#1342
johnomotani wants to merge 7 commits into
shiftedmetric-updatesfrom
calcYupYdown-on-demand

Conversation

@johnomotani

Copy link
Copy Markdown
Contributor

Call calcYUpDown wherever yup/ydown fields are required (it now does nothing if yup/ydown are already present and valid), instead of in Mesh::communicate().

  • makes yup_field and ydown_field members of Field3D mutable
  • adds mutable bool has_yup_ydown to Field3D, so that in Solver::load_vars() we can mark the yup/ydown fields as invalid without deleting them
  • new Field3D& ynextMutable(int dir) const which can be used to get non-const versions of the mutable yup_field and ydown_field from a const Field3D
  • calcYUpDown() methods take a const Field3D as argument

Only calculate yup/ydown fields in ShiftedMetric::calcYUpDown() and
FCI::calcYUpDown, FCI::integrateYUpDown if they are not already set.
Want to allow calling calcYupYdown() within Grad_par operators, etc.,
just before yup/ydown fields are needed.

Allows methods that calculate yup_field/ydown_field to be marked as
const, or take 'const Field3D' as argument.
Now will just call as-needed.
Calls calcYupYdown on the field through its Mesh* pointer.
@ZedThree

Copy link
Copy Markdown
Member

I don't think this is the correct thing to do. calcYupYdown is changing the logical state of a field, therefore it should not be done on a const Field3D.

I think this would only be acceptable if the parallel slices were completely hidden from the user, i.e. they were an implementation detail.

Having a bool hasValidParallelSlices is not a bad idea, but we'd have to be very rigorous about keeping it correct. All the arithmetic-assignment operators would need to mark it as invalid for instance.

@johnomotani

Copy link
Copy Markdown
Contributor Author

Think @ZedThree is right. For example if a user wanted to force a function to use the field-aligned version instead of yup/ydown while using ShiftedMetric everywhere else, this implementation would prevent them from doing so because the const Field3D would be modified 👎

Closing this, but I'll take over the bool hasValidParallelSlices idea to #1176.

@ZedThree
ZedThree deleted the calcYupYdown-on-demand branch October 13, 2023 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants