Remove usage of some deprecated/undocumented functionality

illinois-ceesd / mirgecom

MIRGE-Com is the workhorse simulation application for the Center for Exascale-Enabled Scramjet Design at the University of Illinois.

Other

11 stars 19 forks source link

Remove usage of some deprecated/undocumented functionality #1051

Closed majosm closed 4 weeks ago

majosm commented 1 month ago

Questions for the review:

[x] Is the scope and purpose of the PR clear?
- [ ] ~~The PR should have a description.~~
- [ ] ~~The PR should have a guide if needed (e.g., an ordering).~~
[ ] ~~Is every top-level method and class documented? Are things that should be documented actually so?~~
[ ] ~~Is the interface understandable? (I.e. can someone figure out what stuff does?) Is it well-defined?~~
[ ] ~~Does the implementation do what the docstring claims?~~
[x] Is everything that is implemented covered by tests?
[ ] ~~Do you see any immediate risks or performance disadvantages with the design? Example: what do interface normals attach to?~~

majosm commented 1 month ago

Dropped the removal of https://github.com/illinois-ceesd/mirgecom/blob/03860e762c3d89cc96c856dfd2ac7885eb4d6436/mirgecom/array_context.py#L326 for now. It can't be removed until the default value changes to True in inducer/arraycontext#278. Will submit this as a separate PR later.

majosm commented 1 month ago

Ready for a look. Might be easiest to read commit by commit to see the explanation for each change?

majosm commented 4 weeks ago

Added a few more changes that I missed the first time around @MTCam (including a FIXME relating to a limiter test issue that I noticed). Previous review was up through the commit labeled "remove unused(?) TestSolution".

MTCam commented 4 weeks ago

Added a few more changes that I missed the first time around @MTCam (including a FIXME relating to a limiter test issue that I noticed). Previous review was up through the commit labeled "remove unused(?) TestSolution".

Still looks great to me. Thanks for tackling all this.