Remove twistnum in bspline SPO XML attribute

[ye-luo]

Proposed changes

The existing documentation clearly says avoid using twistnum and I just made one more step by removing it.
Use twist="x.x y.y z.z" input to indicate the exact twist. From now on, all twistnum input will be ignored.
Massive changes are due to updating input files.
I explicit tested complex build.

What type(s) of changes does this code introduce?

• Other (please describe): Simply input tags.

Does this introduce a breaking change?

• No

What systems has this change been tested on?

epyc-server

Checklist

• Yes. This PR is up to date with current the current state of 'develop'

[prckent]

Lets imagine an old input file with twistnum but no twist specified in the XML. Do we abort?

[ye-luo]

First if stopper is desired, I can add it but that requires updating nexus. Users must ensure matching nexus and QMCPACK. In the current way, mostly old input file works as intended with new QMCPACK.

In the massive updates of tests, I only saw two cases where I need to modify twist.

1. graphite performance test, the input was created probably a decade ago. twistnum=2 and I need to add twist="0 0 0.5".
2. All LiH-arb tests, I think you added the tests and the input file was manually created. Thus there was twistnum=0 without a twist specified. The code complains about not being able find the twist. In this case, I added twsit="-0.1 -0.2 -0.3".

In all the input files with a twist input, twistnum="-1" are already specified.

In brief, the only case that breaks is having twistnum input without twist input. The intended twist is not gamma but gamma does exist in the h5. A new QMCPACK picks up gamma instead of the intended twist and keeps running.

[prckent]

Thanks. I was just asking the question. Safe option would be abort with a useful error if twistnum is specified and !=-1. A few minutes work on our part could avoid a bad surprise for someone in future.

NEXUS is an easy update. Yes people should sync, no issues there. (@jtkrogel ?)

[jtkrogel]

OK, first of all, I don't think twistnum should be removed. I thought @ye-luo was going to open a separate issue on this for discussion. There are two reasons for this:

1. Backwards compatibility. Overall, I think we don't prioritize this enough and are overly willing to put users off just because "we don't like something".

2. Nexus is not a minor update and I have been postponing making this update because of the inverted sign issue, as documented in Inconsistent twist directions between electron gas and einspline wavefunctions #1386. The current PR will break all existing workflows.

Instead, we should first make a PR to use twist by default (and not require twistnum="-1" to be provided), and then if it is not provided, use twistnum. Next, #1386 should be fixed in a PR, and we should be sure that HEG, Bspline, and LCAO wavefunctions are using the same convention. Finally, Nexus should be updated in a PR to use the now correct and consistent definition of twists. At this point, twistnum should remain an option for backwards compatibility reasons, but could be removed from the documentation with twist being promoted there instead.

[jtkrogel]

#1386 and #3795 should be resolved first.

[ye-luo]

It is more than "we don't like something". twistnum is not robust against h5 file changes. I can prepare a single h5 for multiple twists even correspond to different supercell and super twists or prepare individual files for each supercell. If we look up by twistnum, it is error-prone. Using twist directly is robust to h5 changes.

Instead, we should first make a PR to use twist by default (and not require twistnum="-1" to be provided), and then if it is not provided, use twistnum.

This already breaks backward compatibility as the input precedence changes. Your concern is to keep a subset of old input running. When twistnum is -1, old input file still runs. So you should be able to update nexus even with this PR in.

About #1386 the sign inversion, I consider that is a different topic and it is better to be addressed as orthogonal as possible. From QMCPACK perspective, the order of making sign change or twistnum change doesn't really matter but probably nexus got hit more on twistnum as it is heavily relied on.

[lshulen]

I'm not sure I support the urgency to get rid of the twistnum capability, although I will not strongly object. I really see specifying the twist versus using twistnum as satisfying two different use cases. If using twistnum directly, you are probably iterating through a list of twists in thinking about twist averaging. In this case from the perspective of preparing the input files, you likely don't care which twist you are looking at, only that you get all of them from the HDF file (also note that this is likely easier to do when you are preparing an input file by hand). On the flip side, you can do much more targeted and clever things when specifying twist directly (for instance running longer when the symmetry is high) but I don't think its the only use case.

[ye-luo]

My second thought on this. I think changing input precedence and keep old files running is better.
We need to remember twistnum has defect "twistnum is not robust against h5 file changes."

[ye-luo]

If neither twistnum nor twist exist, is default to twist=0 0 0 acceptable? The alterrnative is default twistnum=0 which I dislike.

[prckent]

One possibility might be to print a warning ("This is potentially ambiguous. Specifying twist=xyz is preferred.")

We could at least stop creating new files with twistnum.

#1386 needs fixing first... this is a proven cause of wrong results.

[lshulen]

I prefer requiring either twist or twistnum exist to having a default behavior.

[ye-luo]

See #3799