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 different boundary composition plugins on different boundaries #2099

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

Conversation

gassmoeller
Copy link
Member

This sits on top of #2051, and implements the same functionality. The one extension is the fact that there are several compositional fields (compared to only one temperature field).

I figured out a way to allow users to prescribe different boundary conditions for different fields, but I have run into problems with the sparsity pattern of the matrix, because we currently reuse the same matrix block for all compositions. Different boundary conditions would constrain different entries in that block, and so we can not any longer eliminate those entries (because I might need them for another composition).
@tjhei or @bangerth: Did I understand that correctly, and does the change that I made to the sparsity pattern solve this by keeping those matrix entries (core.cc:1088 and 1097)?

@anne-glerum: Could you check if this covers all of the application cases you had in mind? I would also be grateful if you could play around with the boundary conditions and see if something breaks during parsing complicated cases (i.e. some boundary conditions for some fields, others for others, and no boundary conditions for other fields).

Also I still need to figure out what breaks in the discontinuous tests.

@tjhei
Copy link
Member

tjhei commented Feb 19, 2018

@tjhei or @bangerth: Did I understand that correctly, and does the change that I made to the sparsity pattern solve this by keeping those matrix entries (core.cc:1088 and 1097)?

Correct, but the disadvantage is that this will add a lot of nonzero entries in all matrix blocks (especially painful for velocities). I don't know how much of a difference that makes, but I assume we can have 10%+ constrained DoFs in an adaptive computation. @bangerth , what do you think?

I have to play with this a bit, but we should be able to only set this flag for the compositional field matrix where it doesn't matter as much.

@gassmoeller gassmoeller force-pushed the extend_boundary_composition_manager branch 2 times, most recently from 7090fcd to 1c5f6bc Compare March 9, 2018 23:36
@gassmoeller gassmoeller force-pushed the extend_boundary_composition_manager branch from 1c5f6bc to 8d9a09b Compare March 10, 2018 00:12
@gassmoeller gassmoeller changed the title [WIP] Allow different boundary composition plugins on different boundaries Allow different boundary composition plugins on different boundaries Mar 11, 2018
@gassmoeller
Copy link
Member Author

For now I disabled the option to have boundary conditions at a particular boundary for one field, but not another. It is still possible to use different plugins for different fields, but if one field has a prescribed boundary, all of them have to as well. Keeping the constrained dofs seemed to work for the compositions, but then I got errors during the assembly of the Stokes system of the type 'trying to access non-existent matrix entries'. I can look into how to keep constrained dofs for some components only at a later time, but for now we can go forward with this PR (adding the new functionality later will be backward compatible. This is ready from my side.

@gassmoeller
Copy link
Member Author

Let us put this on hold for after the relase. @jdannberg and I discussed a plan to make this change backwards compatible, so it is just another feature. Essentially by introducing a new parameter name, we can support both the old and new versions to set boundary conditions.

@bangerth
Copy link
Contributor

@gassmoeller: What's the status here? Can you rebase and should I then take a look?

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.

3 participants