lammps-users Digest, Vol 145, Issue 1

?try setting restartinfo = 0; in the pair style quip constructor, compile,
and try reading the restart again. ?

I agree that it’s a bug in pair_quip, and unfortunately this doesn’t fix it. It appears that some
sort of flag is set in the restart file, so it tries to call read_restart and gets the Pair method.

Would it not be better if the failure happened during write_restart,
rather than at the read_restart time which might happen after weeks of
running?

?a test could be added, but if somebody uses write_restart after weeks of
running?, the effect would be about the same (if not worse).

Yeah, I agree that this isn’t really a good solution.

if you start arguing like that, you are opening pandora’s box. ?that kind
of logic could be? applies to a gazillion settings in LAMMPS. i would love
to see this happen, but actually with even more rigid requirements, e.g. by
having to pass some bitmasked enums to the constructor and making those
immutable settings. also there are a bunch of other settings, that would
benefit enormously from being only changeable through getter/setter
methods. what LAMMPS does in that regard is conceptually not very different
from using common blocks. unfortunately, it is an uphill battle, since
steve has a kind and generous mind and assumes that everybody is a smart
programmer and tests and double checks everything properly, while i have a
dirty mind and i don’t trust anybody to “do the right thing™”. :wink:
it actually took some effort and a couple of rewrites to get restart
checking stuff into LAMMPS in a way that is compatible with steve’s code
design preferences. as the LOTL he has the final word…

This new test has already opened the Pandora’s box - it is a consistency check. And
moreover it triggers existing (or, arguably, creates new) bugs in existing pair styles.
And even worse, it does so in a way that leads to unreadable restart files. From what I can
tell, the restart file itself has some flag set (NO_PAIR?) which causes the read restart code to
call the potentials’ read_restart method. Setting restartinfo=0 doesn’t seem to change that, because
the flag is already in the restart file. The only way I found to successfully restart is to add a nop
PairQUIP::read_restart. And now these restart files will be forever incompatible with the properly
fixed version of the pair_style, which would presumably have restartinfo=0 and no read_restart
method.

It’s not a that big a deal, but it’d be nice if someone could check all the pair styles and (probably
just using grep) confirm that any that don’t set restartinfo=0 actually have read_restart
and write_restart methods defined. I’d be happy to do that check myself if that’s helpful,
but I’m not sure where to look for all the pair_*.cpp source files (given that some are in
optional packages).

Noam

Here’s the 30 Mar 2018 note on the patch page:

Axel Kohlmeyer (Temple U) made the handling of pair styles that do not store info in restart files more consistent > and transparent, so it’s less confusing for users when they restart a subsequent simulation.

I’m hazy on exactly what was changed for this.But the intent
was to simply make it more clear to users when they
were using a restart file wth a pair style that did
not save all its info in the restart file, e.g. EAM or tersoff.
I.e. they need
to re-specify the pair style command in the restart script.
And protect users from forgetting to do that.

This should not have broken backward compatibility
with old restart files, at least as I recall. We didn’t
flag that it did that on the patch page.

Axel, are you saying there is no simple way for Noam
or others to run with an old restart file? Or there
is just a simple patch to pair_quip.cpp that will allow
that? I.e. something we forgot to do in the 30 Mar patch?

Steve

Here's the 30 Mar 2018 note on the patch page:

> Axel Kohlmeyer (Temple U) made the handling of pair styles that do not
store info in restart files more consistent > and transparent, so it's less
confusing for users when they restart a subsequent simulation.

I'm hazy on exactly what was changed for this.
But the intent
was to simply make it more clear to users when they
were using a restart file wth a pair style that did
not save all its info in the restart file, e.g. EAM or tersoff.
I.e. they need
to re-specify the pair style command in the restart script.
And protect users from forgetting to do that.

This should not have broken backward compatibility
with old restart files, at least as I recall. We didn't
flag that it did that on the patch page.

Axel, are you saying there is no simple way for Noam
or others to run with an old restart file? Or there
is just a simple patch to pair_quip.cpp that will allow
that? I.e. something we forgot to do in the 30 Mar patch?

​(really) old restart files are compatible. as far as i understand, the
problem is with restarts written by buggy pair styles *after* the 30 March
2018 patch. it looks like pair style quip has slipped through your fingers,
when you checked and corrected the styles for the patch.

axel.

?try setting restartinfo = 0; in the pair style quip constructor, compile,
and try reading the restart again. ?

I agree that it’s a bug in pair_quip, and unfortunately this doesn’t fix
it. It appears that some
sort of flag is set in the restart file, so it tries to call read_restart
and gets the Pair method.

Would it not be better if the failure happened during write_restart,
rather than at the read_restart time which might happen after weeks of
running?

?a test could be added, but if somebody uses write_restart after weeks of
running?, the effect would be about the same (if not worse).

Yeah, I agree that this isn’t really a good solution.

​i've looked at the code, and it should be sufficient to make a call to
error->warning() instead of error->all() to make people aware of the bug in
the pair style.
then we still have the check for restarting from pair styles without
restart support in place for pair style without the bug, but ​on the other
hand are more forgiving to buggy pair styles.

axel.