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

[Fluid] Skip replace elements&conditions if geometry-based input modeler is used #12089

Merged
merged 3 commits into from
Feb 22, 2024

Conversation

rubenzorrilla
Copy link
Member

📝 Description
With this PR we will skip the elements and conditions replace done by the ReplaceElementsAndConditionsProcess for the use_input_model_part option in the I/O.

Note that this is aligned with the geometry-based I/O we are targeting with the new modelers.

@KratosMultiphysics/altair as far as I understand this shouldn't break anything in your side. In any case, I'm pinging you to confirm that this doesn't break any specific thing in your custom multistage.

PD: The change of setting the echo level to the strategy after creating it sneaked in 😅. I'd keep it there.

@rubenzorrilla rubenzorrilla self-assigned this Feb 19, 2024
@rubenzorrilla rubenzorrilla requested a review from a team as a code owner February 19, 2024 15:06
@rubenzorrilla rubenzorrilla added this to In progress in Fluid Dynamics Application Project via automation Feb 19, 2024
@rubenzorrilla rubenzorrilla changed the title [Fluid] Skip replace if geometry-based input modeler is used [Fluid] Skip replace elements&conditions if geometry-based input modeler is used Feb 19, 2024
ddiezrod
ddiezrod previously approved these changes Feb 20, 2024
Copy link
Contributor

@ddiezrod ddiezrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not break anything in our side, and the change makes sense to me, approving. 👍

@jcotela
Copy link
Member

jcotela commented Feb 20, 2024

Agreed with @ddiezrod, but I'm adding the API breaker tag

sunethwarna
sunethwarna previously approved these changes Feb 20, 2024
Copy link
Member

@sunethwarna sunethwarna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I also wanted this :) thansk @rubenzorrilla

@rubenzorrilla
Copy link
Member Author

Agreed with @ddiezrod, but I'm adding the API breaker tag

Well, I'm not sure we're breaking the API but changing the default behavior. Let me add the behavior change tag too.

@rubenzorrilla rubenzorrilla added the Behaviour Change changes behaviour but not the API label Feb 21, 2024
@rubenzorrilla rubenzorrilla requested a review from a team as a code owner February 21, 2024 13:15
@rubenzorrilla
Copy link
Member Author

@marcnunezc @inigolcanalejo I think that the test crashing in here is yours. The point is that the primal solution solver in the optimizing is assuming the input model part to be the one from the previous iteration (makes perfect sense). The point is, as far as I understand, that it assumes replacing the elements & conditions (at each iteration!). This is even trickier as the ShapeOptimizationApplication is used, which doesn't follow the conventional AnalysisStage simulation pipeline.

To prevent the test crashing, I added a enforce_elements_and_conditions_replacement member var to the base solver. This is defaulted to False and overwritten in the derived potential solver to True, something that effectively results in the behavior previous to this PR. I make clear that this is IMO a temporary hack since specifying use_input_model_part means use input model part as it is, including elements and conditions. Expecting these to be changed would have been a use_input_geometry setting.

Also pinging @sunethwarna as this uses the ShapeOptimizationApplication, which as far as I know must be substituted by the new OptimizationApplication. Also pinging @Marco1410 as this directly related to his PhD.

Copy link
Member

@sunethwarna sunethwarna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rubenzorrilla rubenzorrilla merged commit ace37c0 into master Feb 22, 2024
16 of 17 checks passed
Fluid Dynamics Application Project automation moved this from In progress to Done Feb 22, 2024
@rubenzorrilla rubenzorrilla deleted the fluid/skip-replace-elements-in-input-model-part branch February 22, 2024 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

4 participants