Skip to content
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

Make it clearer that default_simplex_group is not for external use #339

Open
inducer opened this issue Jun 24, 2022 · 1 comment
Open

Make it clearer that default_simplex_group is not for external use #339

inducer opened this issue Jun 24, 2022 · 1 comment

Comments

@inducer
Copy link
Owner

inducer commented Jun 24, 2022

By deprecating it and adding an underscored version.

By "definition", the function is not precise about what local discretization is chosen, which is a bad idea for code that expects consistent results run-to-run. It's currently used in mirgecom here:

https://github.com/illinois-ceesd/mirgecom/blob/0a43d11bdf5a21552a875636d8e5cefe23f76ab7/mirgecom/discretization.py#L59-L60=

and its use has also leaked into some grudge examples.

cc @MTCam @lukeolson

@MTCam
Copy link
Contributor

MTCam commented Jun 24, 2022

Woops! I'll clean that up in this next PR. I stumbled on that function and thought it looked great because it would continue working even without the recursivenodes package.

fwiw, @lukeolson flagged this change in his review and I was able to convince him it was OK. Regardless, here's the "fix" in mirgecom...
illinois-ceesd/mirgecom@be86451

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants