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

[WIP] Viscoplastic lookup table #5296

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

ryanstoner1
Copy link
Contributor

@ryanstoner1 ryanstoner1 commented Jul 13, 2023

A modification to the visco_plastic material model to optionally set viscosities with lookup tables. These tables are used in place of temperature or pressure conditions by reading a column for dominant phases. What the dominant phase is is checked against a user-provided list of phases of interest.

Update: The goal is to be able to replace the existing analytical phase functions with phase functions read from a lookup table. At the moment this only affects the rheology and anything that is touched by the phase functions. A future PR will add density lookup capabilities as well.

At the moment this is a WIP because:

  • It breaks some visco_plastic_phases tests because some new code is not avoided in the original non-lookup setup.
  • Some assertions are needed to check that users have provided enough information (sufficient information of phases).
  • It implicitly assumes the background field uses a lookup table by iterating over all compositions +1.
  • It needs additional tests.

Open questions are:

  1. Is the structure is correct and consistent with the guidelines?
  2. Are there more concise ways to interface with the lookup table information?
  3. What are some good testing options?

Many thanks to @naliboff @jdannberg @danieldouglas92 for previous discussions! Any additional feedback from those in the community would be appreciated!

Pull Request Checklist. Please read and check each box with an X. Delete any part not applicable. Ask on the forum if you need help with any step.

Describe what you did in this PR and why you did it.

Before your first pull request:

For all pull requests:

For new features/models or changes of existing features:

  • I have tested my new feature locally to ensure it is correct.
  • I have created a testcase for the new feature/benchmark in the tests/ directory.
  • I have added a changelog entry in the doc/modules/changes directory that will inform other users of my change.

Copy link
Member

@gassmoeller gassmoeller left a comment

Choose a reason for hiding this comment

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

I need to continue this tomorrow, but a few comments for now. I think the general idea is correct, but we can optimize things a bit and make it easier to understand.

include/aspect/material_model/utilities.h Outdated Show resolved Hide resolved
include/aspect/material_model/utilities.h Outdated Show resolved Hide resolved
include/aspect/material_model/utilities.h Outdated Show resolved Hide resolved
include/aspect/material_model/visco_plastic.h Outdated Show resolved Hide resolved
include/aspect/material_model/visco_plastic.h Outdated Show resolved Hide resolved
source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
Copy link
Member

@gassmoeller gassmoeller left a comment

Choose a reason for hiding this comment

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

Some more comments, in particular with requests for more documentation. I think I begin to understand what you are doing here, but it will require a few rounds of improvements to get it into a state that is understandable for a non-expert.

source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
Copy link
Contributor

@naliboff naliboff left a comment

Choose a reason for hiding this comment

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

@ryanstoner1 - a few additional comments on top the existing ones, but so far I think the structure you implemented makes sense. At this stage, more documentation in the highlighted places would be helpful to understand what is happening in some places. Thanks for this contribution, it is part of a big step forward in the class of simulations we can do with the visco_plastic model!

source/material_model/utilities.cc Outdated Show resolved Hide resolved
source/material_model/utilities.cc Outdated Show resolved Hide resolved
source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
source/material_model/visco_plastic.cc Outdated Show resolved Hide resolved
@ryanstoner1 ryanstoner1 changed the title Viscoplastic lookup table [WIP] Viscoplastic lookup table Jul 18, 2023
@ryanstoner1 ryanstoner1 changed the title Viscoplastic lookup table Viscoplastic lookup table [WIP] Jul 18, 2023
@ryanstoner1 ryanstoner1 changed the title Viscoplastic lookup table [WIP] [WIP] Viscoplastic lookup table Jul 19, 2023
@anne-glerum anne-glerum added this to In Progress in Fluids and Melts Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Fluids and Melts
In Progress
Development

Successfully merging this pull request may close these issues.

None yet

4 participants