librascal icon indicating copy to clipboard operation
librascal copied to clipboard

Make `compute_loop` inheritable from calculator base class

Open agoscinski opened this issue 4 years ago • 1 comments

Disclaimer: Not important for release, future enhancement, now written to not forget

We should maybe consider to use at some point CRTP for the calculator to make templatable virtual functions for compute and compute_impl

  1. to avoid the repetition of the compute_loop function in each representation
  2. It is easier for non rascal developer to write a representation
  3. to give immediately compiler errors when not implemented (however the later error at the binding level is also informative enough) However this makes the code harder to read at other points, especially to make this work for variadic enum classes takes some extra template structures as in the structure manager to build the cluster_indices_container type. Nothing for this pull request, but should I do a issue for improvement for this?

Remark: If we only allow one enum class to type the calculation the implementation is straightforward

#include <iostream>

enum class Mammal {Dog, Human};
enum class Insect {Fly};

////////////////
// Base class //
// /////////////

// will be "CRTP virtual"
template <typename EnumClass, EnumClass EnumType>
void compute_impl() {}

template <typename EnumClass, EnumClass EnumType>
void compute_loop() {
  compute_impl<EnumClass, EnumType>();
}

// will be "CRTP virtual"
//void compute();

////////////////////////////////
// Calculatior implemenation //
// /////////////////////////////

template <>
void compute_impl<Mammal, Mammal::Dog>() {
  std::cout << "Wuff" << std::endl;
}

template <>
void compute_impl<Insect, Insect::Fly>() {
  std::cout << "Zzzzzzz" << std::endl;
}

void compute(int hyper_p) {
  // different cases, here just one for simplicity
  switch(hyper_p) {
    case(0):
      compute_loop<Mammal, Mammal::Dog>();
    case(1):
      compute_loop<Insect, Insect::Fly>();
  }
}


int main() {
  compute(0);
  compute(1);
  return 0;
};

agoscinski avatar Sep 11 '19 18:09 agoscinski

A simpler (in terms of code modifications) alternative would be to use a variadic templated external function that takes the calculator and mangers as argument, forwards the template parameters for compute_impl and does the loop or not. It seems possible to make it fully generic avoiding some code duplication at the cost of a less clear template argument forwarding.

felixmusil avatar Sep 18 '19 14:09 felixmusil