QMCPACK / qmcpack

Main repository for QMCPACK, an open-source production level many-body ab initio Quantum Monte Carlo code for computing the electronic structure of atoms, molecules, and solids with full performance portable GPU support
http://www.qmcpack.org
Other
298 stars 139 forks source link

Propagation of type aliases through class hierarchies #913

Open PDoakORNL opened 6 years ago

PDoakORNL commented 6 years ago

Getting your type aliases through a base class like QMCTraits becomes problematic when you have a templated base class.

template<class FT>
class OneBodyJastrowOrbitalBsplineSoA :
  public J1OrbitalSoA<FT>
{
  // The following is so we can refer to type aliases(defs) below the
  // templated base class in the object hierarchy
  // Mostly QMCTraits here
  using JBase = J1OrbitalSoA<FT>;
  using RealType = typename JBase::RealType;
  using ValueType = typename JBase::ValueType;
  using GradType = typename JBase::GradType;
  using PosType = typename JBase::PosType;
...
}

In fact getting these type aliases from J1OrbitalSoA has nothing to do with it being a OneBody..'s base class at all. Suggestions? I don't really think QMCTraits belongs in the class hierarchy at all, but sometimes getting a type from your inheritance tree seems more useful.

jtkrogel commented 6 years ago

I actually expect a significant amount of our include spaghetti comes from round about type accesses through mechanisms like these.

PDoakORNL commented 6 years ago

In terms of consistent types I don't think inheritance is a good way to enforce this and to me it looks ineffective in the current code. I'd prefer matching template parameters or they use a global types object. There are too many layers including MI and simply getting the type through inheritance is too vague. It should be crystal clear which RealType or whatever you are using.

For this I prefer to see something like using CTS = CUDAGlobalTypes; then CTS::RealType instead of CudaRealType or RealType

or using QTS = QMCGlobalTypes; QTS::RealType instead of RealType where you have gotten it from somewhere in your large MI inheritance tree.

Additionally this allows a class template like this template<P, V> // P=precisionType, V=valueType

class Whatever
{
using QTS = QMCTypes<P, V>
...
}

This allows generation of however many template classes you need in the executable. i.e. in the actual code there is object code

Whatever<double, double>
Whatever<double,std::complex<double>>
Whatever<single>
Whatever<single, std::complex<single>>

And these are distinct types.

One can write another class template like

template<P, V>
class UsesWhatever
{
using QTS = QMCTypes<P,V>
Whatever<P,V>* myWhatever;
void setWhatever(Whatever<P,V>*);
QTS::ValueType evalSomething(ParticleSet& P, ...);
}
ye-luo commented 6 years ago

QMCTypes should not allow template parameter real/complex. Developers should think harder how to take advantage of ValueType instead of specializing both cases.

PDoakORNL commented 6 years ago

Classes that just want to use global definition of ValueType can simply use the QMCGlobalTypes

They never even need to understand this is a specific template class generated by

QMCGlobalTypes = QMCTypes<PRECISION, VALUETYPE, ...>

This is what you would want to do if you didn't want to write specialized code.

For example the implementation if defined externally

template<P,V>
UsesWhatever::QTS::ValueType UsesWhatever::evalSomething(ParticleSet* P,...)
{
...
QTS::ValueType value = device_spline.evalSomething(std::vector<Pos>);
return value;
}

However if you needed to you could specialize

template<P>
UsesWhatever<P, std::complex<P>>::QTS::ValueType 
UsesWhatever<P, std::complex<P>>::evalSomething(ParticleSet* P,...)
{
... complex specific code

//Below this could be refactored into a non specialized method
QTS::ValueType value = device_spline.evalSomething(std::vector<Pos>);
return value;
}

If defined in the header some of the ugly name qualifications fall away.

ye-luo commented 6 years ago

Current QMCTrailts is your QMCGlobalTypes. Try to improve the existing things instead of having two ways of doing the same thing in the code.

PDoakORNL commented 6 years ago

I just want to comment that when you are refactoring you are trying to make small changes that leave the current functionality intact. This frequently means you're going to devise mechanisms to support legacy code in intermediate steps. I would like to drop the QMCGlobalTypes but until this information propagates through class templates or another mechanism an intermediate where the types are gotten through explicitly using QCT:: versus the current opaque inheritance mechanism.

Until we actually need that legacy code to support something more. We might not actually need something more than that.