-
Notifications
You must be signed in to change notification settings - Fork 17
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
test_gradient
: test warped mesh, overintegration
#329
Conversation
inducer
commented
Feb 23, 2024
•
edited
Loading
edited
- Drop meshmode-repoint
2315fec
to
e3d4e6c
Compare
e3d4e6c
to
2388087
Compare
38f0254
to
6190cea
Compare
6190cea
to
a9ef03e
Compare
discr_tag_to_group_factory[DISCR_TAG_BASE] | ||
) | ||
if DISCR_TAG_MODAL not in discr_tag_to_group_factory and order is not None: | ||
discr_tag_to_group_factory[DISCR_TAG_MODAL] = ModalGroupFactory(order) |
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.
@inducer Question about this: shouldn't the modal group factory be set up in the order is None
case too? Can we retrieve the order from whatever factory is being used for DISCR_TAG_BASE
? (Seeing some failures related to this in mirgecom, hence my asking.)
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.
I agree, it would be ideal if the modal group was set up unconditionally. Unfortunately, I couldn't figure out a robust way to make that happen. Group factories are just callables, so discr_tag_to_group_factory[DISCR_TAG_BASE].order
isn't guaranteed to work. We could try that (and give up if it doesn't work), which would make the situation incrementally better. (The previous version of the code unconditionally did that, so...) But it's not a bulletproof fix either. I would be OK with a PR that does this.
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.
Would it be better to just set it up on the mirgecom side inside create_discretization_collection? We have access to order
there.
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.
(Something like illinois-ceesd/mirgecom#1013.)
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.
Yes, that's a good approach.