Skip to content

Prescribed condition from initial composition#6889

Merged
gassmoeller merged 1 commit intogeodynamics:mainfrom
lhy11009:prescribe_from_initial_composition
Mar 31, 2026
Merged

Prescribed condition from initial composition#6889
gassmoeller merged 1 commit intogeodynamics:mainfrom
lhy11009:prescribe_from_initial_composition

Conversation

@lhy11009
Copy link
Copy Markdown
Contributor

@lhy11009 lhy11009 commented Mar 8, 2026

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.

Prescribe the composition field from the initial composition fields.

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.

@lhy11009
Copy link
Copy Markdown
Contributor Author

lhy11009 commented Mar 8, 2026

Currently, this works for the field methods, and I don't think it works for the particle method. In the test I included, I modified from the composition_passive.prm and prescribed the composition on the left half of the domain. So the left side would stay the same value while the right side is advecting.

t = 0
image

t = 10
image

@lhy11009
Copy link
Copy Markdown
Contributor Author

lhy11009 commented Mar 8, 2026

Ideally, I would want a method that works for the particle method. At least that's something I was thinking when creating the PR. However, I feel this is also useful with the field method.

Copy link
Copy Markdown
Contributor

@danieldouglas92 danieldouglas92 left a comment

Choose a reason for hiding this comment

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

This is great @lhy11009 thanks for this! I only had a few minor comments about cleaning up the test file and the formatting of things, but other then that I think looks great.

Comment thread tests/prescribed_solution_composition_initial_condition.prm
Comment thread include/aspect/prescribed_solution/initial_composition.h
Comment thread source/prescribed_solution/initial_composition.cc
Comment thread source/prescribed_solution/initial_composition.cc
Comment thread source/prescribed_solution/composition_from_initial.cc Outdated
Comment thread tests/prescribed_solution_composition_initial_condition.prm Outdated
@lhy11009 lhy11009 force-pushed the prescribe_from_initial_composition branch from bf0ec1e to 23b9afd Compare March 12, 2026 05:44
@lhy11009
Copy link
Copy Markdown
Contributor Author

Hi @danieldouglas92, Thanks for reviewing my changes.
See my reply on the test file and check the comments I added to the code. Otherwise, I have resolved the conversations.

@danieldouglas92
Copy link
Copy Markdown
Contributor

@lhy11009 the rest of your changes look great! Thanks for addressing them so quickly. If including the cookbook parameter file also causes the model to hang don't worry about it, but I would try adding the include statement to that cookbook file if you haven't already and see what happens.

Copy link
Copy Markdown
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.

The code looks all good @lhy11009, and thanks for the reviews @danieldouglas92. I have some minor comments and questions, but the functionality looks all good.

Comment thread include/aspect/prescribed_solution/initial_composition.h
Comment thread doc/modules/changes/20260311_lhy11009
Comment thread doc/modules/changes/20260311_lhy11009 Outdated
Comment thread tests/prescribed_solution_composition_initial_condition.prm
@lhy11009 lhy11009 force-pushed the prescribe_from_initial_composition branch 2 times, most recently from 55fc501 to ad64145 Compare March 17, 2026 21:52
Comment thread tests/prescribed_solution_composition_initial_condition.prm
@lhy11009 lhy11009 force-pushed the prescribe_from_initial_composition branch 3 times, most recently from ec34707 to ec7459b Compare March 25, 2026 03:23
@lhy11009
Copy link
Copy Markdown
Contributor Author

Previously, there was a building issue, I think that is related to a missing header.
I added the following line to source/prescribed_solution/initial_composition.cc file:
#include <aspect/geometry_model/interface.h>
Hopefully, this addresses the issue.

Copy link
Copy Markdown
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.

Yes, that header was missing if you do not precompiler the header files (which we usually do, but some systems may not allow that, that is why one of the testers doesnt do it).

Looks all good except for my comment about the test.

subsection Visualization
set Interpolate output = false
set Time between graphical output = 0.1
set Output format = gnuplot
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You didnt add any output files for the test. This way we wont notice if the feature breaks. Please add one (one should be enough).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I have added two gunplot files

@lhy11009 lhy11009 force-pushed the prescribe_from_initial_composition branch from ec7459b to b4e4652 Compare March 26, 2026 16:44
@gassmoeller
Copy link
Copy Markdown
Member

/rebuild

Copy link
Copy Markdown
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.

Ready to merge when the testers are done. (the one failing tester is unrelated)

@gassmoeller
Copy link
Copy Markdown
Member

/rebuild

@gassmoeller
Copy link
Copy Markdown
Member

All good to merge now.

@gassmoeller gassmoeller merged commit 6a53304 into geodynamics:main Mar 31, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants