-
Notifications
You must be signed in to change notification settings - Fork 15
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
tentative fix to ALL exit bug #245
Conversation
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.
Just a comment: I find it a bit unintuitive to pass the number of timesteps to the constructor because why should the decomposition actually care about that info (yes for the ALL bugfix but actually it shouldn't). The other way to get to the information of the number of timesteps would be by querying the xmlconfig
during the readXML()
step. However, I suppose then you would miss the number of steps when it is passed via the command line. For this you could query global_simulation->getNumTimesteps()
. On the other hand, accessing the singleton is always an ugly workaround, but already widely used throughout ls1. So maybe an ok-ish solution in readXML()
could be
- check
if global_simulation != nullptr
and query that if available - otherwise query
xmlconfig
.
In the end, I feel all of the options are ugly in their own way, so decide for yourself what you prefer.
I agree with@FG-TUM that the domain decomposition should not require the number of time steps to simulate. Note also that ls1 could be run using a time limit instead a fixed number of steps. |
I agree that all workarounds are ugly. I personally wanted to avoid accessing the singleton since I'd like to be able to phase out its use in the future. I'll take a look at the initialization procedure for the ALL object and see if I can fix it in a more logical way, however that will take some time. |
@cniethammer Does that feature (still) exist? It is not mentioned in |
From a quick look into the source I see that, e.g., the |
These changes do not fix the actual bug -> closing |
Description
Simulation now exits gracefully when ALL update frequency < number of simsteps
Resolved Issues
How Has This Been Tested?
Same test case as outlined in issue