search

geodynamics / aspect

A parallel, extensible finite element code to simulate convection in both 2D and 3D models.
https://aspect.geodynamics.org/
Other
224 stars 235 forks source link

Keep old checkpoints, even if current model has not been restarted. #608

Closed jperryhouts closed 8 years ago

jperryhouts commented 8 years ago

The conditions for creating a backup of the previous snapshot are a bit weird. If a model has been restarted (ie parameters.resumecomputation == true): every snapshot from that point on copies the old resume.* to resume..old Otherwise: no resume._.old files are ever created

I think the intention was to only do this once at the beginning of the resumed model in case it fails right away, but the variable which is intended to record that this copy has happened is local to the function where it happens, and so it gets overwritten each time and thus the copy happens during every subsequent snapshot.

I think that copy is actually useful behavior though, and these restart.*.old files should be created every time. It provides a nice fallback for when something goes wrong while writing a snapshot.

tjhei commented 8 years ago

Did you see my recent commit https://github.com/geodynamics/aspect/commit/aba6303b3efb54834257c5faffc7fc53c086c652? I think the logic is correct now: We start doing the move a) after the first snapshot or b) after a resume.

tjhei commented 8 years ago

This of course means that we won't keep an old file it it belongs to a different (older) run. Is that what you want to achieve?

jperryhouts commented 8 years ago

Ah, cool. I was on @jdannberg 's melt branch and that commit hadn't been merged there yet. Thanks!

By the way, that function seems a bit magical. I guess static variables won't be reinitialized, but it's not obvious that's what's intended here just by reading the function.

Anyway, thanks Timo! Closing this issue now.

bangerth commented 8 years ago

Yes, static variables are only initialized the first time around. Feel free to propose a patch with a comment that explains what wasn't clear to you in the beginning :-)

jperryhouts commented 8 years ago

Since it works now it's unlikely that someone will bother to audit that function in the future, but I guess additional comments can't hurt. :)

bangerth commented 8 years ago

25 years of experience with software tells me that that code will be refactored at one point or other sometime in the next 5 years (as all other code will as well)...