WaveFunctionComponent API is misleading and inconsistent.

[PDoakORNL]

Is your feature request related to a problem? Please describe. WFC's are in the current wavefunction design basically state machines. The base class interface obscures and confuses this, the member function names and documentation are just confusing.

The significant effect of many functions is actually updating object state. The names of functions don't reflect their actual purpose or what they do.

Examples:

 /** evaluate the value of the WaveFunctionComponent from scratch
   *  @param P  active ParticleSet
   *  @param G Gradients, \f$\nabla\ln\Psi\f$
   *  @param L Laplacians, \f$\nabla^2\ln\Psi\f$
   *  @return the log value
   *
   */
  virtual LogValueType evaluateLog(const ParticleSet& P,
                                   ParticleSet::ParticleGradient_t& G,
                                   ParticleSet::ParticleLaplacian_t& L) = 0;

Actually the normal mode of getting the LogValue of a WFC is to access the data member logvalue not call this function. This actually recomputes most of the state of most WFC and provides an exception to the usual semantics for a WFC's LogValue. Later on in the getValue() function there is another definition for the value of a wfc that will not always match the value that evaluateLog returns.

There is infact another way to accomplish the same state transformation as evaluateLog by calling evaluateGL with a flag to get a fresh evaluation of GL.

This key API needs a real clean up.

Describe the solution you'd like TBD

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context Add any other context or screenshots about the feature request here.

[prckent]

I agree with this. I think that the current situation is also why adding a new wavefunction component is so non-obvious, despite the guide in the manual. The complexities are also visible the unit test setup.

This is probably the most important API in the code, or at least the most fundamental to running real space QMC efficiently. There will always be a state machine here, and making an accessible code requires it be properly surfaced and documented.

The function definitions are currently incomplete (and could therefore be interpreted as wrong).

I think a big improvement would be to first properly document and perhaps rename the existing functions to make clear in the name and documentation where some state is assumed or updated. e.g. Perhaps evaluateVGLUpdateState(), evaluateVGLFromScratch() etc.

In the evaluateLog example, the log is not very relevant. The "from scratch" is more relevant, plus that the full value, 1st and 2nd derivatives will be computed, since this affects the computational cost and prerequisites.

I would not want to see any non-trivial structural changes ahead of thoroughly understanding+documenting the current situation.

[ye-luo]

For WaveFunctionComponent base class, we need to consolidate the definition and improve the documentation. This is this is a non-stop effort as we evolve the code.

"There is infact another way to accomplish the same state transformation as evaluateLog by calling evaluateGL with a flag to get a fresh evaluation of GL." this is implementation detail in the derived class. We do give flexibility in derived class and the way things are handled in a particular class doesn't mean the base class should be adjusted accordingly.

The recompute flag of evaluateGL is a hint, it doesn't mean every derived class need to recompute from scratch.

Algorithms are state-machines. It needs to exist in one form or another. I think we need a way in documentation to show how a driver interacts with TWF/WFC APIs.