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

Physical av #735

Merged
merged 519 commits into from
Apr 6, 2023
Merged

Physical av #735

merged 519 commits into from
Apr 6, 2023

Conversation

anderson2981
Copy link
Contributor

@anderson2981 anderson2981 commented Aug 25, 2022

Production feeder for physical artificial viscosity.

Add a new transport class that implements artificial viscosity as an additive term to the physical viscosity.

This requires an additional parameter included with the fluid state, the smoothness. The smoothness is derived from the discretization and needs to be available to the transport class to compute the new viscosity term. We have added the smoothness field to dv.

Adds a new example which doubles as an integrated test to illustrate how to setup and use the new transport class and tests that the solution works and stays in the acceptable numerical range with physical AV enabled.

based on work in #717

Questions for the review @lukeolson :

  • 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?
  • Is everything that is implemented covered by tests?
    • Not everything; a strict correctness test is missing, but until one is identified we decided to continue.
  • Do you see any immediate risks or performance disadvantages with the design?
    • MTC: There is extra smoothness data now added to every run, regardless of the use of AV, significantly affecting our memory and communication footprints (smoothnesses are communicated). We can fix this later (Physical AV extra data #867).

@MTCam MTCam added enhancement New feature or request Production Feeder Feeds the production branch labels Aug 25, 2022
@MTCam MTCam mentioned this pull request Aug 25, 2022
MTCam and others added 25 commits October 31, 2022 08:28
Date:   Fri Nov 4 04:35:10 2022 -0500

    Update CI scripts to use driver scripts to drive tests.
MTCam and others added 18 commits April 1, 2023 22:20
* Add option to turn off inviscid terms in CNS operator.

* Deflake8

* partial update

* tweak mengaldo BC code

1) separate individual aspects of BCs (adiabatic, slip, etc.) into their
   own component classes
2) use *_plus for values that will be used in conjunction with *_minus
   values in numerical flux functions, and *_bc for values that will be
   used to compute flux directly

* use grad_temperature_bc instead of grad_temperature_plus

* remove duplicated inviscid/viscous flux methods

* make inviscid/viscous flux internal variables

* change grad_momentum_bc to grad_velocity_bc in _SlipBoundaryComponent and use it in SymmetryBoundary

* deprecate SymmetryBoundary

* add comment about energy for inviscid

* only use _ldg_bnd_flux_for_grad in mengaldo BCs

* rename state_plus to state_plus_inviscid and explain why

* don't pass callbacks to base class constructor unless they're expected to be used

* add FIXME

* add some docstrings

* simplify cv_plus/cv_bc construction by using cv.replace

* add replace_fluid_state function and use it to further simplify state construction in BCs

* tweak some docstrings

* add note

* restore SymmetryBoundary docstring to avoid unused reference error in doc build

* Reset CI

* Deawkwardize doc wording.

* Make clear that lewis must be an array

* Add option to turn off inviscid terms in CNS operator. (#856)

Co-authored-by: tulioricci <[email protected]>

* Try to fix doc err.

* Adding variable spec. diff to PowerLawTransp (#772)

Co-authored-by: Mike Campbell <[email protected]>
Co-authored-by: Matthias Diener <[email protected]>

* Partial update

* Refactor Mengaldo: Isothermal, and Adiabatic for clarity

* Update doublemach to provide its own boundary.  This AV is deprecated, preparing for removal

* Matts changes update 1.

* Add Matts replace fluid state helper

* Use smoothness consistent

* Switch to mengaldo bcs

* Update doc with boundary interface change.

* Undo test changes

* Random cleanup

* Remove duplicate func.

* Update docs

* Add another Poinsot ref, fix doc err?

* Extricate AV-specific junk from fluid boundaries.

* Extricate AV from fluid boundaries.

* Use production MANIFEST.in, try to fix gmsh ish with doublemach.

* Update lingering bcs

* Update per final review comments

* Update outdated comment

---------

Co-authored-by: Matthew Smith <[email protected]>
Co-authored-by: tulioricci <[email protected]>
Co-authored-by: Tulio <[email protected]>
Co-authored-by: Matthias Diener <[email protected]>
Base automatically changed from mengaldo-bcs to main April 3, 2023 23:14
mirgecom/eos.py Outdated
# MJA, it doesn't appear that we can have a None field embedded inside DV,
# make a dummy smoothness in this case
if smoothness_mu is None:
smoothness_mu = 0. * cv.mass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any (compile) risks with initializing smoothness_mu from cv.mass? Should we use zeros or zeros_like directly from the array context?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern is used pervasively. While we prefer the zeros_like facility (which was created exactly for this type of thing), this pattern is not (or has not presented itself as) a compilation issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we think of this? 1425245

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks better -- there are several places where this is used with smoothness_*

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only found a couple places in eos. I suppose where they are not passed (i.e. for inviscid-only and runs with AV off), that setting them to the same zeros is OK. e1ad33f

@MTCam MTCam mentioned this pull request Apr 6, 2023
@MTCam MTCam merged commit 1c18917 into main Apr 6, 2023
@MTCam MTCam deleted the physical-av branch April 6, 2023 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Production Feeder Feeds the production branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants