[CoSimulationApplication] Adding FluxWrapper and FluxIO
#11523
Conversation
loumalouomega
added
Cleanup
Applications
FastPR
loumalouomega
added
Enhancement
and removed
Cleanup
FastPR
loumalouomega
changed the title
FluxWrapper and FluxIO
|
Hm there is lots of unused code, will review over the weekend Btw I think there is even wrong syntax such that the file cannot even be imported |
Okay
What do you mean?, I refactored the code that was given to me (cleaning up) and a priori a fixed some import issues |
| # Check for a subprocess call | ||
| world_rank = 0 | ||
| if KM.IsDistributedRun() : | ||
| world_data_comm = KM.ParallelEnvironment.GetDataCommunicator("World") | ||
| world_rank = world_data_comm.Rank() | ||
|
|
||
| # Prevent any launch of a FluxServer from a subprocess | ||
| if world_rank != 0: | ||
| return |
There was a problem hiding this comment.
this should not be necessary due to the usage of the _GetDataCommunicator function. Not sure why it was added
There was a problem hiding this comment.
Me neither, as I said, I have not wrote this
There was a problem hiding this comment.
To my understanding, it can be changed to world_rank=_GetDataCommunicator().Rank() but let me confirm it with the author
There was a problem hiding this comment.
I think the entire block is not needed any more, I implemented this (solvers not running on some MPI-procs) on a global level
I suggest to remove it and try
applications/CoSimulationApplication/python_scripts/solver_wrappers/external/flux_wrapper.py
Outdated
Show resolved
Hide resolved
applications/CoSimulationApplication/python_scripts/solver_wrappers/external/flux_wrapper.py
Outdated
Show resolved
Hide resolved
applications/CoSimulationApplication/python_scripts/solver_wrappers/external/flux_wrapper.py
Outdated
Show resolved
Hide resolved
| if hasattr(self, 'flux') : | ||
| # Close the MultiPhysic session | ||
| self.flux.CloseMultiPhysics() | ||
| # Save and close the Flux project | ||
| self.flux.SaveProject() | ||
| self.flux.CloseProject() |
There was a problem hiding this comment.
should this also be called in Finalize?
If so, then perhaps refactor to a separate fct
There was a problem hiding this comment.
Okay, but I don't know the details
There was a problem hiding this comment.
I don't think we should call it twice in finalize and also in del but let me ask the author.
There was a problem hiding this comment.
meant to call it in Finalize instead of del, as in my experience del is not reliable
but perhaps a flag would be useful, e.g. flux_was_closed which is set in Finalize. If for some reason the run crashes, and only del is called, then you could execute the closing of flux depending on this flag
There was a problem hiding this comment.
maybe you could also use atexit instead of del
applications/CoSimulationApplication/python_scripts/solver_wrappers/external/flux_wrapper.py
Outdated
Show resolved
Hide resolved
applications/CoSimulationApplication/python_scripts/solver_wrappers/flux_io.py
Outdated
Show resolved
Hide resolved
applications/CoSimulationApplication/python_scripts/solver_wrappers/external/flux_wrapper.py
Outdated
Show resolved
Hide resolved
…ppers/external/flux_wrapper.py Co-authored-by: Philipp Bucher <[email protected]>
Co-authored-by: Philipp Bucher <[email protected]>
Co-authored-by: Philipp Bucher <[email protected]>
|
@pooyan-dadvand there are two comments from @philbucher that I don't know how to handle, can you take a look? |
|
I am closing this PR as Flux team feels that the wrapper is not ready yet to be added here. |
📝 Description
Includes #11524
This PR introduces a new CoSimulation wrapper called
FluxWrapperthat serves as a dedicated Kratos wrapper for theFluxsolver.NOTE: This wrapper class was not developed by me, the details I can provide are limited.
The contents of the commit include the following changes:
FluxWrapperclass that extendsCoSimulationSolverWrapper.FluxIOclass that extendsCoSimulationIO🆕 Changelog
FluxWrapperandFluxIO