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

checkpointing test with LA::distributed::Vector #12823

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tjhei
Copy link
Member

@tjhei tjhei commented Oct 13, 2021

This adds a failing test for checkpointing using SolutionTransfer in parallel with LA::distributed::Vector. The problem is that we set values in

cell->set_dof_values_by_interpolation(*it_input,
*(*it_output),
fe_index);

and then call compress(Insert):
(*it)->compress(::dealii::VectorOperation::insert);

which is not supported using this vector type.

@tjhei
Copy link
Member Author

tjhei commented Oct 13, 2021

@kronbichler here it is. Do you have a suggestion? I know that we set values more than once for a continuous FE, so using Add is not an option without keeping track of how many times we set the values.
Naive suggestion: allow compress(Insert) to work by ignoring NaN and fill all ghost values with NaN before setting values.

@kronbichler
Copy link
Member

This reveals a more general problem that has bothered me for a long time. Depending on the VectorOperation passed to compress, we perform different actions in either our own vector or external classes. I like the suggestion of using NaN as some initial condition to the vector entries. This is a step we would need to do with the Vector type before we start the operation inserting the elements, mirroring the compress() call in the end. In other words, I think for a clean solution we would need to use Vector::set_neutral_element(vector_operation) before the loop over cells, and call compress(vector_operation) in the end with the same vector_operation. This is because also other VectorOperation choices, e.g. min or max, are actually not happy with our default value of operator=(0.), as setting all entries to 0 is only the neutral element for add. min would need std::numeric_limits<Number>::max() and max would need -std::numeric_limits<Number>::max() in order to make consistent arguments. What do think about this general solution? If we agree, we could work towards this goal in several steps, as the current PR is rather a symptom of a larger underlying problem.

@tjhei
Copy link
Member Author

tjhei commented Oct 26, 2021

Yes, this is exactly what I was thinking about. It makes sense that we can generalize to min/Max, etc..
Is this too big of a change? I think this could be quite useful...

@bangerth
Copy link
Member

Yes, I think too that that is the right direction to go!

@kronbichler
Copy link
Member

OK, I'm glad we agree here. How should we proceed? I could work on this functionality for LA::d::Vector first on a separate branch, and then we try to use it here?

@tjhei
Copy link
Member Author

tjhei commented Oct 28, 2021

I appreciate your help! If you want to help, maybe we start with the implementation of set_neutral_element(Insert) and then I can test it here.
Instead of introducing a new function, we could also reuse zero_out_ghost_values by adding the vectoroperation as an argument (with default Add). The name is okay in my opinion (it is the "zero" or neutral element for the operation).

@tamiko tamiko marked this pull request as draft July 4, 2023 18:05
@tamiko tamiko removed the WIP ⚠️ label Jul 4, 2023
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.

None yet

4 participants