Disable backflow optimization, Remove EinsplineSet and its derived classes

[ye-luo]

Proposed changes

Cannot find any tests. The only feature that relies on this is backflow during optimization. backflow without optimization doesn't need EinsplineSet.
EinsplineSetBuilder will be refactored and moved to QMCWaveFunctions/BsplineFactory and tested.

In addition, I did some cleaning in the EinsplineBuilder. To reduce confusion, I use kpoints when referring to primitive cell since it is from DFT calculations and keep using twists when referring to super cell which is QMC calculation.

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

• Other (please describe): Remove mostly unused and untested code.

Does this introduce a breaking change?

• Yes

What systems has this change been tested on?

laptop

Checklist

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

[ye-luo]

Test this please

[ye-luo]

Test this please

[prckent]

Do you think these were some of the relevant code paths for https://journals.aps.org/prl/abstract/10.1103/PhysRevLett.104.185702 ? There seems to have been multiple routes to handle APW type schemes.

[prckent]

Did you look at the state of testing for " backflow during optimization", and can it still run after this change? (Clearly there should not need to be any dependencies...)

[prckent]

This is good cleanup. 3K lines removed :-) . Will be happy to merge provided the backflow situation is clear. Maybe we need more testing there.

[ye-luo]

MuffinTin has been removed in #3678. There is no "backflow during optimization" test currently which needs use_old_spline="yes" input. In this PR, old spline implementation cannot be accessed anymore and thus no "backflow optimization" support.

[prckent]

From your comment this means that backflow won't be usable for science. A better strategy will be needed or this is equivalent to dropping backflow support.

[ye-luo]

From your comment this means that backflow won't be usable for science. A better strategy will be needed or this is equivalent to dropping backflow support.

We can dig out the code and port it into new classes once a scientific use shows up. Or users can keep using old releases. More harm to keep outdated classes in my view.

[prckent]

Couple of points, one specific to this PR, the other more general about how to approach these feature removals.

First, please note what would be needed to restore backflow optimization? What is the specific functionality we were using in the old classes that is not present in the modern ones? This note is needed for future revisiting, and perhaps if it can be readily described we might be able to fix it sooner.

Second, from a user perspective this PR is effectively a feature removal since it would discourage anyone from using backflow. It would have to be announced in the release notes. Backflow is a headline feature for historical reasons, and some other codes don't have it. However, I doubt very many people (1? 2?) are using it or have plans to, particularly outside of model systems. On this basis we could even go further and consider removing all the backflow code - it certainly slows us to keep outdated code around. However, someone could also rightly argue that backflow should be explored more, since it has been used so little for ab initio work...

For this PR, and for any future feature removals, we should establish a pattern of first posting on the QMCPACK Google group as well as the QMCPACK Slack to find out if anyone is using it/has plans to, and then make plans to accommodate them if needed. That way we can avoid burning any existing users or surprising potential ones.

[jtkrogel]

Perhaps check with @mmorale3 and @Paul-St-Young

[Paul-St-Young]

I see the arguments for removing backflow as a feature. It would greatly reduce the burden of code maintenance.
Although, could this be done after the v4 transition rather than before?

There are currently multiple groups actively using backflow:

1. CCQ QMC group
2. UIUC Ceperley group
3. Switzerland/Israel group

We are all using the legacy driver, so it's OK to not port backflow to the batched drivers.
It would be nice if we could get a final release of a well tested legacy driver with full features.

[ye-luo]

I see the arguments for removing backflow as a feature. It would greatly reduce the burden of code maintenance. Although, could this be done after the v4 transition rather than before?

There are currently multiple groups actively using backflow:

1. CCQ QMC group

2. UIUC Ceperley group

3. Switzerland/Israel group

We are all using the legacy driver, so it's OK to not port backflow to the batched drivers. It would be nice if we could get a final release of a well tested legacy driver with full features.

We all wanted a perfect version for legacy driver users before moving on but it won't work in practice. Anytime we spend on legacy driver is taken from the time we could spend on batched drivers. Is 3.16 release good for the need of above group? If there is a bug, it is easier to fix directly on top of 3.16 release than keeping legacy code around in v4.

[ye-luo]

Couple of points, one specific to this PR, the other more general about how to approach these feature removals.

First, please note what would be needed to restore backflow optimization? What is the specific functionality we were using in the old classes that is not present in the modern ones? This note is needed for future revisiting, and perhaps if it can be readily described we might be able to fix it sooner.

High order partial derivatives of SPOSet are needed to be called by determinants with backflow. Those implementations need to be added back.

Second, from a user perspective this PR is effectively a feature removal since it would discourage anyone from using backflow. It would have to be announced in the release notes. Backflow is a headline feature for historical reasons, and some other codes don't have it. However, I doubt very many people (1? 2?) are using it or have plans to, particularly outside of model systems. On this basis we could even go further and consider removing all the backflow code - it certainly slows us to keep outdated code around. However, someone could also rightly argue that backflow should be explored more, since it has been used so little for ab initio work...

The current implementation of backflow won't go far as system size grows. It needs to be sufficiently reworked to run p-by-p in batched drivers. Such less used features needs to be maintained preferably by direct users. They need to participate the development. Thus we need to grow developer base by waking them up.

For this PR, and for any future feature removals, we should establish a pattern of first posting on the QMCPACK Google group as well as the QMCPACK Slack to find out if anyone is using it/has plans to, and then make plans to accommodate them if needed. That way we can avoid burning any existing users or surprising potential ones.

"accommodate" them should not simply mean we spend our time to provide a feature to users. Users need to pay some tax or they get involved in the development directly. The mentioned groups are very welcome to engage with us. They can provide example runs, write unit tests, or even do needed porting work. That is more healthy model for development.
We can say in the release that this feature is temporarily blocked until we restore it in batched drivers. Users don't need to understand how we blocked the feature.

[prckent]

"accommodate" them should not simply mean we spend our time to provide a feature to users

Indeed. We should have the conversation in each case to avoid surprise and get more contributions as practical. Ideally we can encourage/help users to do porting work, particularly for obscure features. The project can't scale if it is the same few people doing the implementation work.

In the present case I think it would help to get some test runs and tests made while the files for the runs are most readily available. We should at least have an optimization test that works now and will fail with a sensible message after this PR in the batched drivers (we can set it to expect fail). @Paul-St-Young Can you encourage the other two groups who have published with the code to contribute? If we have a few electron but representative sample of their physics included in the test suite there is a good chance of keeping it alive. Also, if they modified the code it would be ideal to have the change contributed back.

[Paul-St-Young]

it is easier to fix directly on top of 3.16 release than keeping legacy code around in v4.

I agree it makes no sense to carry legacy stuff in v4, but how about making a 3.17 release with a bit TLC in the legacy parts?

Can you encourage the other two groups who have published with the code to contribute?

Yes, let me try to contact them. I'm sure we can contribute some examples and tests.
The problem with code contribution is that we use the legacy components (see below).

Users need to pay some tax or they get involved in the development directly.
f they modified the code it would be ideal to have the change contributed back.

I couldn't agree more with the need for a reciprocal relationship.
I am more than eager to contribute, but my understanding is that you no longer accept changes to the legacy driver and estimator.

[prckent]

I agree it makes no sense to carry legacy stuff in v4, but how about making a 3.17 release with a bit TLC in the legacy parts?

If you can be specific, of course we'll consider this. We are a bit overdue for a 3.x release but can't wait too long since it can hold up changes such as those in this PR.

Can you encourage the other two groups who have published with the code to contribute?

Yes, let me try to contact them. I'm sure we can contribute some examples and tests. The problem with code contribution is that we use the legacy components (see below).

There is no problem here :-) e.g. If you want some reassurance of what is in 3.17, we need a test.

[Paul-St-Young]

I found the legacy version of magnetization density from this rejected PR (#4135) more user-friendly than the new batched version.
I also made a few improvements to legacy estimators in my branch that I am happy to contribute.
It would be nice to restore the EnergyDensity estimator (#4628).
And I would really appreciate a flag to dump walkers at some interval, which is super useful for observable development, among other things. (whether using the TraceManager or not)

[prckent]

Thanks Paul. These are all good feature improvement requests. Contributions are welcome :-) Several of these are sorely needed in any version of the code, e.g. the dumpwalkers feature would have quite a few uses.

What I am going to propose we do is make a new release with the current development branch that will serve as the starting point for a long term support branch for v3. We will continue development towards v4 independently on the main branch. We will therefore be able to avoid holding up refactoring and cleanup work needed for v4 while still being able to take contributions and accept bug fixes to v3. ( Will get input to this on Slack )

[prckent]

Since this PR was made we have released a v3.17.0 and v3.17.1 and setup a long term support branch support/v3.17.x . Contributions ranging from small tweaks to full feature contributions are welcome, as discussed. The current PR will be merged into develop which is now fully focused on improvements and developments for v4.

[prckent]

Test this please

[prckent]

LGTM