-
Notifications
You must be signed in to change notification settings - Fork 250
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
base: master
Are you sure you want to change the base?
[SeveralApps] Improving and adding missing python scripts for adaptive remeshing #4558
Conversation
I have a couple of comments related to the imports |
But run the examples in the examples repository. If your concern is that
the MeshingApplication is imported I can tell you that without it cannot
find the MMG_process.py
El dom., 31 mar. 2019 19:10, Philipp Bucher <[email protected]>
escribió:
… I have a couple of comments related to the imports
I would push directly to this branch (tomorrow) if this is ok for you
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#4558 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ATEMgLPOF8PtfviyamCtoOxMP6nEaFq2ks5vcOvngaJpZM4cUaEU>
.
|
Ok I will check |
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? Note: |
I will |
It works fine |
|
||
while self.time < self.end_time: | ||
self.time = self._GetSolver().AdvanceInTime(self.time) | ||
# We reinitialize if remeshed previously |
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.
Shouldn't the re-meshing be done always in the same moment? These if statements can be easily misunderstood...
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.
Nope, you may be interested in doing the remeshing during the simulation, not before, and then you need to reinitialize the whole simulation
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.
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.
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.
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 ... Also the current solution is too messy imo to go mainstream |
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 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 |
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... |
I did not say to continue as done here, we discussed in @KratosMultiphysics/implementation-committee to start over again and rethink the solution |
@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 |
@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. |
@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... |
Please read carefully...
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. |
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)