Add delayed update

[ye-luo]

Closes #239. Unit test and "check_wfc -f Det" are added.

[ye-luo]

test this please

[ye-luo]

test this please

[ye-luo]

Test this please

[ye-luo]

Test this please

[ye-luo]

I think the added entanglements are necessary for real QMC algorithm and QMCPACK needs.

Ownership of SPO. When the trial wavefunction is Slater-Jastrow type, it may contains determinants using SPOs. When the whole wavefunction doesn't contain Slater determinant, then there is no SPO. Then SPO needed to be owned by determinants. The code is more modular and composable. If SPO is owned outside determinants, we have to deal with a null pointer or size 0 vector to handle SPO. This is not a clean route and indicates a non-generic design.

EinsplineSPO or base class SPOSet. Even though that the miniQMC only supports spline SPO, the design needs to be flexible to be incorporated back to QMCPACK. So hard coding EinsplineSPO and fulfilling only EinsplineSPO needs should be avoided. I'm not saying SPOSet is perfect and believe some call interfaces needs to be updated.

SPOSet using ParticleSet instead of particle positions. SPOSet represents all the SPOs including LCAO and hybrid representation. Those calculation needs electron ion-distances. ParticleSet owns the particle position and distance tables. Whether to use or not all the data owned by ParticleSet depends on the derived SPO class. I would argue that ParticleSet is better than position only because ParticleSet is more generic. SPOSet::evaluate(ParticleSet elecs, int iel) is more generic than diverged EinsplineSPO::evaluate(Pos_t R) and LCAO::evaluate(Pos_t R, Pos_v ions) or LCAO::evaluate(Pos_t R, Pos_v displ_ions) Passing ParticleSet should not stop you doing anything achievable with positions only.

[PDoakORNL]

I don't understand the argument about the harmonic oscillator basis, but if you are just going to appeal to the main source code everytime miniqmc simply isn't useful for design discussions.

I think it's much clearer if the actual terms needed to evaluate the function are in the function signature. If that requires specialized code that is fine there is an important detail that at least the einspline SPO doesn't have.

So to me the second is much better. The idea that elecs would somehow change the ion positions used to evaluate the LCAO is counter intuitive to me.

[PDoakORNL]

Anyway make the other changes and I will approve, the ParticleSet discussion is too big for this PR.

[ye-luo]

I made a mistake harmonic oscillator is a SPO not a WaveFunctionComponent. I correct all the words and removed harmonic oscillator as example.

I would consider the old design as one of the candidates of the new design not being excluded.

I think for virtual functions and functions of templates classes, it is better to have call APIs as generic as possible. If call APIs and classes both need specialization, we cannot solve an all-to-all problem.

[PDoakORNL]

Seems more like a specialization on an isDependentOnIonPositions trait.

[ye-luo]

@PDoakORNL Could you correct my following broken code using isDependentOnIonPositions trait? The following code doesn't compile because EinsplineSPO and LCAO only have one of the two evaluate interface. Please modify and make it working.

Deteriminant<class SPOType>
{
void ratioGrad()
{
   SPOType spo;
   if(isDependentOnIonPositions<SPOType>::value)
      spo.evaluate(Pos_t R, Pos_v ions); // LCAO only has this interface
   else
      spo.evaluate(Pos_t R); // EinsplineSPO only has this interface
}
}

[PDoakORNL]

#include<vector>
#include<type_traits>
#include<iostream>

class LCAOSet
{
 public:
    void evaluate(double pos, std::vector<double>& ions) { std::cout << "ions matter\n"; }
};

class EinsplineSet
{
 public:
    void evaluate(double pos) { std::cout << "what's an ion?\n"; }
};

template <typename t>
struct isDependentOnIonPositions : public std::false_type {};

template<>
struct isDependentOnIonPositions<LCAOSet> : public std::true_type {};

template<class SPOT>
struct StateIShouldNotHave;

template<>
struct StateIShouldNotHave<EinsplineSet>
{
    double pos = 1.0;
};

template<>
struct StateIShouldNotHave<LCAOSet>
{
  double pos = 1.0;
  std::vector<double> ions = {1.0, 2.0, 3.0};
};

template<class SPOT>
class Determinant
{
  SPOT spo;
  StateIShouldNotHave<SPOT> state;
 public:
    template<typename U = SPOT,
       typename std::enable_if<isDependentOnIonPositions<U>::value, std::nullptr_t>::type = nullptr>
    void ratioGrad()
    {
      spo.evaluate(state.pos , state.ions);
    }

    template<typename U = SPOT,
       typename std::enable_if<!isDependentOnIonPositions<U>::value, std::nullptr_t>::type = nullptr>
    void ratioGrad()

    {
      spo.evaluate(state.pos);
    }
};

int main()
{
  Determinant<LCAOSet> det_lcao;
  Determinant<EinsplineSet> det_einsplineset;

  det_einsplineset.ratioGrad();
  det_lcao.ratioGrad();
}

Even xlc++ can do it

Edit: I'd rather the enable_if in the parameter list

[ye-luo]

@PDoakORNL Thank you. I expected this complexity. If I have not just one call inside ratioGrad but very long function body, having it repeated the whole function because of 1 call difference, I feel, is not really worth it. If we have a third SPOSet which need another interface, the ratioGrad need a third version. Hopefully you can consider evaluate(const ParticleSet&) and we don't need to maintain this complexity.

The code pattern you showed is very powerful to handle data types or functions from libraries which is often not allowed to be modified. Having traits and SFINAE works very well.