-
Notifications
You must be signed in to change notification settings - Fork 186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor vector invariant advection #2820
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excellent
make sure to bump the version before merging
This makes progress towards It will also allow us to more easily implement new reconstruction schemes to use with vector invariant advection... should we come up with any. |
Should we have both |
I guess they are identical. Maybe we can change the |
|
||
required_halo_size(scheme::WENOVectorInvariant{N}) where N = N + 1 | ||
required_halo_size(scheme::WENOVectorInvariant) = required_halo_size(scheme.advection) + 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💪
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't it scheme.scheme
or something now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe should be scheme.vorticity_reconstruction
or something
Closing in favor of #2852 |
swap orders of WENO reconstruction and VectorInvariant advection so instead of
we write
This means that now
VectorInvariant
works with any reconstruction scheme. I.e., it is possible to write alsoCloses #2271