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

[SeveralApps] Improving and adding missing python scripts for adaptive remeshing #4558

Open
wants to merge 196 commits into
base: master
Choose a base branch
from

Conversation

loumalouomega
Copy link
Member

This adds some missing scripts fro adaptive remeshing. Specially the analysis for FluidDynamicsApplication

(This is in order to make the examples on the Examples repository to work and specially in order to make easier to correct/update in the future)

@philbucher
Copy link
Member

I have a couple of comments related to the imports
I would push directly to this branch (tomorrow) if this is ok for you

@loumalouomega
Copy link
Member Author

loumalouomega commented Mar 31, 2019 via email

@philbucher
Copy link
Member

Ok I will check

@philbucher
Copy link
Member

I cleaned the imports, there were many unused ones

I also checked the examples and pushed some corrections, now they should work also without importing the MeshingApp

I don't have mmg yet, could you please test the cases you are concerned about?
The tests in Kratos are running

Note: import python_script will stop working at some point, the safe way is to add the application-name like this: from kratos.AppName.python_script as py_script
=> this also imports the application

@loumalouomega
Copy link
Member Author

I cleaned the imports, there were many unused ones

I also checked the examples and pushed some corrections, now they should work also without importing the MeshingApp

I don't have mmg yet, could you please test the cases you are concerned about?
The tests in Kratos are running

Note: import python_script will stop working at some point, the safe way is to add the application-name like this: from kratos.AppName.python_script as py_script
=> this also imports the application

I will

@loumalouomega
Copy link
Member Author

I cleaned the imports, there were many unused ones
I also checked the examples and pushed some corrections, now they should work also without importing the MeshingApp
I don't have mmg yet, could you please test the cases you are concerned about?
The tests in Kratos are running
Note: import python_script will stop working at some point, the safe way is to add the application-name like this: from kratos.AppName.python_script as py_script
=> this also imports the application

I will

It works fine


while self.time < self.end_time:
self.time = self._GetSolver().AdvanceInTime(self.time)
# We reinitialize if remeshed previously
Copy link
Member

@rubenzorrilla rubenzorrilla Apr 1, 2019

Choose a reason for hiding this comment

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

Shouldn't the re-meshing be done always in the same moment? These if statements can be easily misunderstood...

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, you may be interested in doing the remeshing during the simulation, not before, and then you need to reinitialize the whole simulation

Copy link
Member

@rubenzorrilla rubenzorrilla Apr 1, 2019

Choose a reason for hiding this comment

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

But this means that you might do the re-meshing before and after the InitializeSolutionStep(). When does this have sense?
If the re-meshing is to be done once, and we want to call it as a process, I'd rather go for a interval-type solution as we use in the processes. Then you won't probably need to check this kind of casuistic.

Copy link
Member

@rubenzorrilla rubenzorrilla left a comment

Choose a reason for hiding this comment

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

Why it is needed to have a different analysis stage for remeshing?

@loumalouomega
Copy link
Member Author

Why it is needed to have a different analysis stage for remeshing?

Because if you remesh you need to reinitialize everything, maybe we can integrate the remeshing into the base analysis stage (if ModelPart is modified then reinitilize the whole system)

@philbucher
Copy link
Member

Why it is needed to have a different analysis stage for remeshing?

Because if you remesh you need to reinitialize everything, maybe we can integrate the remeshing into the base analysis stage (if ModelPart is modified then reinitilize the whole system)

hm I am not sure abt this ...
This is quite specific to problems with a mesh
What abt DEM, MPM etc?

Also the current solution is too messy imo to go mainstream

@rubenzorrilla
Copy link
Member

Why it is needed to have a different analysis stage for remeshing?

Because if you remesh you need to reinitialize everything, maybe we can integrate the remeshing into the base analysis stage (if ModelPart is modified then reinitilize the whole system)

This is my point, I'd go for a common (as common as possible as @philbucher says) rather than adding new analysis stage files to all the apps.

@loumalouomega loumalouomega requested review from a team as code owners September 27, 2020 07:46
Vicente Mataix Ferrándiz added 2 commits September 27, 2020 13:04
@roigcarlo
Copy link
Member

@loumalouomega Does it make sense to keep this alive or should be redone?

@loumalouomega
Copy link
Member Author

@loumalouomega Does it make sense to keep this alive or should be redone?

Should be redone after reaching a consensus. But I need to have such consensus before starting to code, meanwhile the branch can stay alive

@rubenzorrilla
Copy link
Member

Just mentioning that, at list in a very fist implementation, we can consider the adaptive remeshing as a first (or intermediate) stage of a multistage simulation, so we do it operation-based.

OFC this is a limitation as we cannot do adaptive remeshing while running the simulation, but it think that, in the vast majority of our use cases we can consider the adaptive remeshing "a la multilevel simulation". If in the future we see that this becomes in a limitation, we can always implement one at the stage-level (and not in the upper one).

@loumalouomega
Copy link
Member Author

Just mentioning that, at list in a very fist implementation, we can consider the adaptive remeshing as a first (or intermediate) stage of a multistage simulation, so we do it operation-based.

OFC this is a limitation as we cannot do adaptive remeshing while running the simulation, but it think that, in the vast majority of our use cases we can consider the adaptive remeshing "a la multilevel simulation". If in the future we see that this becomes in a limitation, we can always implement one at the stage-level (and not in the upper one).

I don't think remeshing is a multistage simulation, because maybe you don't know when remeshing is necessary. In the @KratosMultiphysics/implementation-committee we discussed like this problem have some similarities with the adaptive NR, as you need to restore previous solution before try a gain with a smaller time step.

@rubenzorrilla
Copy link
Member

Just mentioning that, at list in a very fist implementation, we can consider the adaptive remeshing as a first (or intermediate) stage of a multistage simulation, so we do it operation-based.
OFC this is a limitation as we cannot do adaptive remeshing while running the simulation, but it think that, in the vast majority of our use cases we can consider the adaptive remeshing "a la multilevel simulation". If in the future we see that this becomes in a limitation, we can always implement one at the stage-level (and not in the upper one).

I don't think remeshing is a multistage simulation, because maybe you don't know when remeshing is necessary. In the @KratosMultiphysics/implementation-committee we discussed like this problem have some similarities with the adaptive NR, as you need to restore previous solution before try a gain with a smaller time step.

I agree, but as I put in my comment, this would be a first step towards an eventual final implementation. The point is that by doing it in two steps, the majority of implementations would have been done, and we'll need to only fit them into the analysis stage. Also note that if you want to do multilevel simulation the remeshing is, at least in my opinion, an intermediate stage of a multistage simulation, so it wouldn't be a waste of effort.

@loumalouomega
Copy link
Member Author

Just mentioning that, at list in a very fist implementation, we can consider the adaptive remeshing as a first (or intermediate) stage of a multistage simulation, so we do it operation-based.
OFC this is a limitation as we cannot do adaptive remeshing while running the simulation, but it think that, in the vast majority of our use cases we can consider the adaptive remeshing "a la multilevel simulation". If in the future we see that this becomes in a limitation, we can always implement one at the stage-level (and not in the upper one).

I don't think remeshing is a multistage simulation, because maybe you don't know when remeshing is necessary. In the @KratosMultiphysics/implementation-committee we discussed like this problem have some similarities with the adaptive NR, as you need to restore previous solution before try a gain with a smaller time step.

I agree, but as I put in my comment, this would be a first step towards an eventual final implementation. The point is that by doing it in two steps, the majority of implementations would have been done, and we'll need to only fit them into the analysis stage. Also note that if you want to do multilevel simulation the remeshing is, at least in my opinion, an intermediate stage of a multistage simulation, so it wouldn't be a waste of effort.

But you may don't know when remeshing must be done, you may set a remeshing criteria and maybe your criteria says mesh is fine, so no remeshing, I think it is better to think in a generic adaptive analysis, this will be useful for cases such the NR, remeshing and maybe others.

@rubenzorrilla
Copy link
Member

rubenzorrilla commented Nov 17, 2022

Just mentioning that, at list in a very fist implementation, we can consider the adaptive remeshing as a first (or intermediate) stage of a multistage simulation, so we do it operation-based.
OFC this is a limitation as we cannot do adaptive remeshing while running the simulation, but it think that, in the vast majority of our use cases we can consider the adaptive remeshing "a la multilevel simulation". If in the future we see that this becomes in a limitation, we can always implement one at the stage-level (and not in the upper one).

I don't think remeshing is a multistage simulation, because maybe you don't know when remeshing is necessary. In the @KratosMultiphysics/implementation-committee we discussed like this problem have some similarities with the adaptive NR, as you need to restore previous solution before try a gain with a smaller time step.

I agree, but as I put in my comment, this would be a first step towards an eventual final implementation. The point is that by doing it in two steps, the majority of implementations would have been done, and we'll need to only fit them into the analysis stage. Also note that if you want to do multilevel simulation the remeshing is, at least in my opinion, an intermediate stage of a multistage simulation, so it wouldn't be a waste of effort.

But you may don't know when remeshing must be done, you may set a remeshing criteria and maybe your criteria says mesh is fine, so no remeshing, I think it is better to think in a generic adaptive analysis, this will be useful for cases such the NR, remeshing and maybe others.

Please, read my comment carefully. Again, I know what you meant, but your situation is much more complex and involving that the one I'm considering. That's why I would consider mines as a first step, which we can use to develop some missing functionalities. In a second step, I'd consider yours, but not starting from scratch but from the utilities we developed to materialize first. Note that my and your cases do not exclude each other and MUST coexist.

EDIT: In any case this is my observation of how I would proceed if I were the one implementing it. You can do it the way you want, but note that proceeding like this resulted in a PR which got stuck for more than 3 years...

@loumalouomega
Copy link
Member Author

EDIT: In any case this is my observation of how I would proceed if I were the one implementing it. You can do it the way you want, but note that proceeding like this resulted in a PR which got stuck for more than 3 years...

I did not say to continue as done here, we discussed in @KratosMultiphysics/implementation-committee to start over again and rethink the solution

@pooyan-dadvand
Copy link
Member

@KratosMultiphysics/technical-committee would like to understand better the conceptual change before implementation details and see if we can divide and attack each step one by one.

@loumalouomega
Copy link
Member Author

@KratosMultiphysics/technical-committee would like to understand better the conceptual change before implementation details and see if we can divide and attack each step one by one.

I need to rethink everything in first place and probably start from scratch

@rubenzorrilla
Copy link
Member

@KratosMultiphysics/technical-committee has been discussing about this again.

Considering the complexity of having a standarized design to be able to remesh "at the solver/stage level", for instance, after each time step, and also taking into account this is not an usual operation we suggest to proceed as suggested by @rubenzorrilla.

This is aligned with the ideas developed by @loumalouomega @ddiezrod and @pooyan-dadvand at Altair. This means to consider the adaptive remeshing as a modeler. Note that now that we have multi-stage support, we can do a first stage to solve a problem and calculate the metric and then pass this to a second stage that does the adaptive refinement at the modeler level.

In the future, when the multistage conception is production-ready, we can discuss about its technical limitations and discuss about the "internal" one upon requirement.

Having said this, we would suggest to close this PR and start a fresh new one.

@loumalouomega
Copy link
Member Author

@rubenzorrilla I like the idea of using a modeler, but I think is not the solution for the issue, at least the way you presented the solution. If we perform a simulation of 1000 steps (for example), and we remesh every 5 steps, do you mean I need to divide the simulation in 200 stages?, for me this is not reasonable. It could be a solution if the design of the stage is flexible enough and can be nested without needing to define and indefined number of stages. This means that maybe we need to redesign the multistage strategy...

@rubenzorrilla
Copy link
Member

Please read carefully...

In the future, when the multistage conception is production-ready, we can discuss about its technical limitations and discuss about the "internal" one upon requirement.

What the @KratosMultiphysics/technical-committee discussed, agreed and meant with the previous comment is that in the case you need to do what you post you'll probably end up doing a custom adaptive remeshing analysis stage. Note that doing a physics-agnostic generic solution for this is extremely complicated (and admittedly not a current priority for the project).

Hence, we're pretty convinced that doing an inter-stage implementation, even though it has of course limitations like the one you posted, it's a reasonable way in terms of value/effort to start working on this. Note that an eventual implementation at the stage level will perfectly coexist with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Applications Consensus Consensus has been reached about the current issue Enhancement Implementation Committee Consensus The implementation committee has reached consensus on what to do next Kratos Core Python
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

9 participants