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

Wrong Assertion for nearest projection mappings #135

Closed
davidscn opened this issue Jul 10, 2020 · 5 comments · Fixed by #248
Closed

Wrong Assertion for nearest projection mappings #135

davidscn opened this issue Jul 10, 2020 · 5 comments · Fixed by #248
Assignees
Milestone

Comments

@davidscn
Copy link
Member

At the time we implemented the nearest projection mapping, we asserted, that it is not used in FSI simulations, since there, the Solid participant needs to provide connectivity.
However, after introducing #125 , this is not valid any more, since we can use nearest-projection mappings for stress data.

openfoam-adapter/Adapter.C

Lines 150 to 159 in b1d80ef

if(interfacesConfig_.at(i).meshConnectivity == true )
{
adapterInfo(
"Mesh connectivity is not supported for FSI, as, usually, "
"the Solid participant needs to provide the connectivity information. "
"Therefore, set provideMeshConnectivity = false. "
"Have a look in the tutorial README or the Github wiki for detailed information. "
,"warning");
return false;
}

Either we should remove this Assertion completely or just raise a warning, that this is only relevant for FSI in combination with stress data (which is currently only supported by our deal.II adapter).

@uekerman uekerman changed the title Wrong Assetion for nearest projection mappings Wrong Assertion for nearest projection mappings Jul 11, 2020
@MakisH
Copy link
Member

MakisH commented Jul 11, 2020

Good point.

This is currently a warning + trigger exit. A rewording of the warning and removing the return false should do the job.

@davidscn
Copy link
Member Author

Yes sure, there is not that much work included. The main question for me is here: Should we raise a warning like 'you can just use FSI and nearest projection in combination with stress data' or should we just remove it completely, assuming that people know, what they are configuring?

@MakisH
Copy link
Member

MakisH commented Jul 11, 2020

I would not rely on the sanity of the user, there are many reasons on why one can misconfigure this. If they misconfigure it, I am not sure if preCICE will complain, so they will falsely think they are doing an NP mapping. In any case, the issue will be obscure.

We could still make the error more specific and move into the write/read methods of Force:

void preciceAdapter::FSI::Force::write(double * buffer, bool meshConnectivity, const unsigned int dim)

@davidscn
Copy link
Member Author

I am not sure if preCICE will complain

In case no connectivity is provided, preCICE raises a warning, that it falls back to a NN mapping.

We could still make the error more specific and move into the write/read methods of Force:

I agree in general, but let's try to place it somewhere, where it is not asserted in every iteration i.e. not in the write methods directly but e.g. in the constructor, if possible.

@MakisH
Copy link
Member

MakisH commented Jul 12, 2020

In the constructor it would be the best, but it currently does not have enough information. In any case, we should refactor this part in the future to not need to pass the connectivity flag through every function.

We currently have such condition checks in many write/read functions (look e.g. here), so I would not worry too much, if we can't avoid it.

@MakisH MakisH added this to the v1.2.0 milestone Sep 19, 2022
@MakisH MakisH self-assigned this Sep 19, 2022
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 a pull request may close this issue.

2 participants