-
Notifications
You must be signed in to change notification settings - Fork 35
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
Change initialize sequence #648
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good to me. I think that the transfer is subordinate to the operators, so the order seems more reasonable to me anyway.
this->initialize_operators(); | ||
|
||
this->initialize_transfer_operators(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the problem is it could happen that we need the transfer operators in order to initialize operators. The concrete example I am thinking of is a velocity or displacement dof-vector that needs to be interpolated to coarser grids. If the present function is called with initialize_preconditioners = true
, this might lead to a bug because the operators are not initialized completely.
So you modify again the constraints and the matrix-free objects in Right now, I am somewhat against this change because it seems to me that the problem is only solved for your case by "mis-using" the implementation or certain functions at other places. We could think about making the |
Yes, correct. I know this is not the intended use but it needs to be done for agglomeration.
A virtual |
I did not fully understand this. Generally, I would like to not make everything protected (which has been private before). As I explained above, the problem that I see with the suggested change is that it only works because the logic is kind of "mis-used" at another place. |
I have a bit of a chicken-egg problem in my code because of this initialize sequence change in #599
The problem is that my operator modifies the constraints and therefore also the matrix-free object and setting up the transfer correctly works only after the final matrix-free object is set up.
https://github.com/exadg/exadg/pull/599/files#r1401973099 states that this sequence change should not affect anything and also does not change the logic of initializing things dependent on
initialize_preconditioners
.