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

allow a different list of assemblers for each advection field #5233

Merged

Conversation

jdannberg
Copy link
Contributor

It is useful to allow different advection fields to have different assemblers (i.e., we have different advection methods like advection field, diffusion field etc.) and not all of them require all assemblers. In cases where a signal is used to replace assemblers, it may even be necessary to only do that for specific fields and not for all of them.

This PR makes the advection system assembers a vector of vectors (with each compositional field having their individual vector of advection assemblers). At the moment, all fields still get the same assemblers, to test that this change does not break any functionality. The next step would be to only include the assemblers that are needed in each list (in a separate PR).

The change in the entropy benchmark shows how this could look like.

I also want to see which tests this breaks to see where I still have to fix the assemblers.

For all pull requests:

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.

The code looks correct, but lets see what the testers say.

@jdannberg
Copy link
Contributor Author

The tester thinks it's correct 🙂

@gassmoeller gassmoeller merged commit f663f48 into geodynamics:main Jul 9, 2023
6 checks passed
@jdannberg jdannberg deleted the multiple_advection_assemblers branch July 10, 2023 01:29
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.

None yet

2 participants