Skip to content

Modernize Mirgecom#360

Closed
thomasgibson wants to merge 16 commits intomainfrom
thg/remove-eager
Closed

Modernize Mirgecom#360
thomasgibson wants to merge 16 commits intomainfrom
thg/remove-eager

Conversation

@thomasgibson
Copy link
Copy Markdown
Contributor

@thomasgibson thomasgibson commented May 23, 2021

This PR does the following:

  1. Removes the use of grudge.discretization.EagerDiscretization. Why? Because grudge is, for the most part, agnostic to whether arrays are eager, gpu, or lazy. It is the job of arraycontext to sort out the behavior of arrays, not grudge. Moreover, this uses the more stable (non-deprecated) interface of grudge.
  2. Updates deprecated imports from meshmode to the new arraycontext package. This also includes the swapping of arguments for functions like thaw.
  3. Renames wave-eager-x.py -> wave-x.py. This is because of item 1. When lazy evaluation comes in, the only thing that should really change (from mirgecom's perspective) is the use of a new lazy array context. Apparently CI didn't like this.

Comment thread examples/mixture-mpi.py Outdated
Comment thread mirgecom/diffusion.py Outdated
@thomasgibson
Copy link
Copy Markdown
Contributor Author

So my plan is to wait until inducer/grudge#74 is merged before this PR is "ready." However, I think we should keep commenting about the user interface until we're all happy with it.

@thomasgibson thomasgibson marked this pull request as draft May 24, 2021 18:40
@inducer
Copy link
Copy Markdown
Contributor

inducer commented May 24, 2021

Thanks @thomasgibson for taking this on! (Just did a quick scroll here, no huge red flags spotted.)

@inducer
Copy link
Copy Markdown
Contributor

inducer commented May 24, 2021

Unsubscribing... @-mention or request review once it's ready for a look or needs attention.

@thomasgibson
Copy link
Copy Markdown
Contributor Author

Haven't forgotten about this PR; I would prefer to have inducer/grudge#122 in first

@MTCam
Copy link
Copy Markdown
Member

MTCam commented Sep 22, 2021

Haven't forgotten about this PR; I would prefer to have inducer/grudge#122 in first

Hey @thomasgibson , I think this addresses some portion of #509 ?

@thomasgibson
Copy link
Copy Markdown
Contributor Author

Haven't forgotten about this PR; I would prefer to have inducer/grudge#122 in first

Hey @thomasgibson , I think this addresses some portion of #509 ?

Yes it indeed does! It will take care of removing references to discr (and allow us to remove EagerDGDiscretization all together)

@lukeolson lukeolson mentioned this pull request Jun 6, 2022
15 tasks
@MTCam MTCam closed this Jun 25, 2022
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

Successfully merging this pull request may close these issues.

4 participants