Unification of Grid/qcd/utils/Sp2n.h and Grid/qcd/utils/SUn.h

[chillenzer]

Might be possible though it is not that likely. Potentially, at least some functionality is duplicated and we can extract that as free functions or the like? Assess if such introduced coupling is advantageous in the long run.

[edbennett]

Something to think about—let's say at a later date we/someone else wanted to add support for SO(N). How much work can we save from that process?

[chillenzer]

Well, probably the same amount that we would need to put in now for abstracting this. You know premature optimization is the root of all... how was that saying... apple trees? I always thought it was kind of strange that all apple trees use the same root.

Admittedly, there might be a tiny overhead for getting familiar with the relevant parts of the code again... but as we will sure add great documentation for all the features we implement that will be done in no time. =)

[edbennett]

Mm, and then you have SOn.h, SUn.h, and Sp2n,h, and three copies of mostly the same code to maintain…

(Funnily enough, most apples trees do use rootstock different to the tree growing out of it, as they are grafted to give better yields.)

[chillenzer]

Is it "mostly the same code"? It is probably the same interface but under the hood it would look quite different, wouldn't it?

(That is an amazing coincidence and a github issue is one of the last places I would have expected to acquire such knowledge!)

[edbennett]

Ah, yes, my mistake, the diffs do look horrible in fact.

Part of the problem is I'm also trying to find a technical solution to a social problem (as is frequently the case). Specifically, if someone comes along and changes the interface to SUn.h and doesn't notice that Sp2n.h is there, then Sp2n will break, which will not be noticed (due to the test suite not being automated, modulo what the main repository does with TeamCity which I can't see) and will surprise the next person to try and clone Grid to use Sp(2N). Having the code next to it will at least hint to the person making the changes that they should also fix Sp(2N), or alert the people using Sp(2N) that this needs doing.

I guess an alternative would be to formalise an interface and then have both SUn and Sp2n implement that interface. Does inheriting from a pure virtual base class still introduce potential performance problems from indirection?

[chillenzer]

Of course. That is the main problem with code duplication.

I'm not precisely sure if a sufficient amount of final keywords could fix the performance penalty in cases where you never actually use the interface (which is not what you want to do) but as soon as you use the interface (which would be a huge change in the code BTW) you definitely have one. But more importantly, inheritance is more about runtime polymorphism (which is why it comes with runtime performance penalty). What we want, is compile-time polymorphism and that is implemented via concepts/SFINAE and templates.

You could potentially misuse concepts/SFINAE constructs to implement requirements for a static interface. They would trigger as soon as someone tries to use (as opposed to implement) the class with a wrong interface. But having them around is probably a higher maintenance burden than not.

In fact, I guess that the compiler would complain already if you actually change and use the interface of the groups inconsistently because it will fail to compile some template instances.

[chillenzer]

Just had a closer look at the problem. Some insights:

1. The interface is indeed mostly the same. The implementations obviously need to be very different at some points but quite a bit of the auxiliary functionality is (almost) identical, too.
2. The generators of different type are specific to the group but should be private anyways. This might interfere with some of the testing and we should address this trade-off in detail.
3. There is a bit of functionality in SU(N) that does not appear in Sp(2N) but that seems to be rather a matter of not needing it as opposed to not being applicable.

So, I guess in theory this would make it a perfect example of template specialisation... if it weren't for the additional template parameter ncolour. C++ is very picky about partial template specialisations; you can partially specialise classes and member variables but you cannot partially specialise functions including member functions.

What we want to do is still allowed but kind of ridiculed: We could provide an abstract template declaration (including some definitions)

template<GroupName name, int ncolour>
class GaugeGroup {...};

but the naive approach to partial specialisation will fail

template<int ncolour>
int GaugeGroup<SU, ncolour>::generator(...){...}

Instead, C++ requires a partial class specialisation to happen

template<int ncolour>
class GaugeGroup<SU, ncolour> {...}

in order to have a full (as opposed to partial) template member function of the partially specialised class afterwards. This implies that we would still have to copy the complete interface to all partial specialisations and it is very much allowed to do any omissions or additions in there, i.e. we would not guard against incompatible interfaces.

Other parts of the code often solve this kind of problem with macros but I'm not really sure if that would increase maintainability for such a large chunk of code.

[chillenzer]

Okay. I compiled three approaches into a MWE to solve this:

1. Partial class specialisation as described above
2. Separating the group specific functionality into a separate struct partial specialisations of which are so unique that it is fine to create complete copies.
3. Variant of 2. using static functions which makes it a bit more obvious how much can happen at compile time but might be a tiny bit less intuitive for calls to group-unique functions.

From my perspective the three approaches increase in elegance with respect to the common functionality but also in annoyance with respect to the use functionality that is unique to a group (like OmegaInvariance in this example).

#include <iostream>

enum GroupName { SU, Sp };

/////////////////////////////
/////////////////////////////
namespace partial_class_specialisation {

template <GroupName name, int ncolour>
struct GaugeGroup {
  int f(int x);
};

template <int ncolour>
struct GaugeGroup<SU, ncolour> {
  int f(int x);
};

template <int ncolour>
struct GaugeGroup<Sp, ncolour> {
  int f(int x);
  int OmegaInvariance(int x);
};

template <int N>
int GaugeGroup<GroupName::SU, N>::f(int x) {
  return x + N;
}

template <int N>
int GaugeGroup<GroupName::Sp, N>::f(int x) {
  return N * x;
}

template <int N>
int GaugeGroup<GroupName::Sp, N>::OmegaInvariance(int x) {
  return x + 20;
}

}  // namespace partial_class_specialisation

/////////////////////////////
/////////////////////////////
namespace separate_group_specific_member {

template <GroupName name, int ncolour>
class GroupSpecificFunctions;

template <int ncolour>
struct GroupSpecificFunctions<SU, ncolour> {
  int f(int x) { return x + ncolour; }
};

template <int ncolour>
struct GroupSpecificFunctions<Sp, ncolour> {
  int f(int x) { return x * ncolour; }
  int OmegaInvariance(int x) { return x + 20; }
};

template <GroupName name, int ncolour>
struct GaugeGroup {
  GroupSpecificFunctions<name, ncolour> group_specifics;
  int f(int x) { return group_specifics.f(x); }
};
}  // namespace separate_group_specific_member

/////////////////////////////
/////////////////////////////
namespace separate_group_specific_static {

template <GroupName name, int ncolour>
class GroupSpecificFunctions;

template <int ncolour>
struct GroupSpecificFunctions<SU, ncolour> {
  static int f(int x) { return x + ncolour; }
};

template <int ncolour>
struct GroupSpecificFunctions<Sp, ncolour> {
  static int f(int x) { return x * ncolour; }
  static int OmegaInvariance(int x) { return x + 20; }
};

template <GroupName name, int ncolour>
struct GaugeGroup {
  int f(int x) { return GroupSpecificFunctions<name, ncolour>::f(x); }
};
}  // namespace separate_group_specific_static

int main(void) {
  {
    using namespace partial_class_specialisation;
    GaugeGroup<GroupName::SU, 3> su3;
    GaugeGroup<GroupName::Sp, 4> sp4;
    std::cout << "SU(3): " << su3.f(4) << "\n";
    std::cout << "Sp(4): " << sp4.f(4) << "\n";
    std::cout << "Sp(4) OmegaInvariance: " << sp4.OmegaInvariance(1) << "\n";
  }
  {
    using namespace separate_group_specific_member;
    GaugeGroup<GroupName::SU, 3> su3;
    GaugeGroup<GroupName::Sp, 4> sp4;
    std::cout << "SU(3): " << su3.f(4) << "\n";
    std::cout << "Sp(4): " << sp4.f(4) << "\n";
    std::cout << "Sp(4) OmegaInvariance: "
              << sp4.group_specifics.OmegaInvariance(1) << "\n";
  }
  {
    using namespace separate_group_specific_static;
    GaugeGroup<GroupName::SU, 3> su3;
    GaugeGroup<GroupName::Sp, 4> sp4;
    std::cout << "SU(3): " << su3.f(4) << "\n";
    std::cout << "Sp(4): " << sp4.f(4) << "\n";
    std::cout << "Sp(4) OmegaInvariance: "
              << GroupSpecificFunctions<Sp, 4>::OmegaInvariance(1) << "\n";
  }
  return 0;
}

[chillenzer]

PS: Of course, we would use typedefs in the end to restore the interface we know and love.

[chillenzer]

PPS: Would of course foster as well as benefit from #11.

[chillenzer]

PPPS: As we would have to change the SU(N) class significantly, we should probably check with Peter once we decided on the most promising candidate.

[chillenzer]

Okay. I finally remembered why there is no partial function template specialisation and it turned out to be the solution: Partial function template specialisation would overlap with function overloading which is usually even more powerful and can be used instead. Here is a very neat solution:

#include <iostream>
#include <type_traits>

enum class GroupName { SU, Sp };

namespace function_overloading {

template <GroupName name>
class GroupNameType {};

template <GroupName name>
struct is_sp {
  static const bool value = false;
};

template <>
struct is_sp<GroupName::Sp> {
  static const bool value = true;
};

template <GroupName name, int ncolour>
struct GaugeGroup {
  int f(int x) { return f(x, GroupNameType<name>()); }
  int f(int x, GroupNameType<SU>) { return x + ncolour; }
  int f(int x, GroupNameType<Sp>) { return x * ncolour; }

  template <GroupName dummy_name = name,
            typename = std::enable_if_t<is_sp<dummy_name>::value>>
  int OmegaInvariance(int x) {
    return x + 20;
  }
};
}

int main(void) {
  {
    using namespace function_overloading;
    GaugeGroup<GroupName::SU, 3> su3;
    GaugeGroup<GroupName::Sp, 4> sp4;
    std::cout << "SU(3): " << su3.f(4) << "\n";
    std::cout << "Sp(4): " << sp4.f(4) << "\n";
    std::cout << "Sp(4) OmegaInvariance: " << sp4.OmegaInvariance(1) << "\n";
  }
  return 0;
}

It's beautiful, isn't it?

[chillenzer]

PS: Concerning code organisation, the main GaugeGroup template needs all the declarations (like int f(int x, GroupNameType<SU>)...) afaik but the actual definitions of such could be separated out, e.g. into own files SUn.h etc.

[LupoA]

I should mention that OmegaInvariance is only needed for testing. If unifying the groups would be easier, it can be probably moved into the tests that need it. Or it can be defined for each group, but SUn would not need it.

[LupoA]

Okay. I finally remembered why there is no partial function template specialisation and it turned out to be the solution: Partial function template specialisation would overlap with function overloading which is usually even more powerful and can be used instead. Here is a very neat solution:

#include <iostream>
#include <type_traits>

enum class GroupName { SU, Sp };

namespace function_overloading {

template <GroupName name>
class GroupNameType {};

template <GroupName name>
struct is_sp {
  static const bool value = false;
};

template <>
struct is_sp<GroupName::Sp> {
  static const bool value = true;
};

template <GroupName name, int ncolour>
struct GaugeGroup {
  int f(int x) { return f(x, GroupNameType<name>()); }
  int f(int x, GroupNameType<SU>) { return x + ncolour; }
  int f(int x, GroupNameType<Sp>) { return x * ncolour; }

  template <GroupName dummy_name = name,
            typename = std::enable_if_t<is_sp<dummy_name>::value>>
  int OmegaInvariance(int x) {
    return x + 20;
  }
};
}

int main(void) {
  {
    using namespace function_overloading;
    GaugeGroup<GroupName::SU, 3> su3;
    GaugeGroup<GroupName::Sp, 4> sp4;
    std::cout << "SU(3): " << su3.f(4) << "\n";
    std::cout << "Sp(4): " << sp4.f(4) << "\n";
    std::cout << "Sp(4) OmegaInvariance: " << sp4.OmegaInvariance(1) << "\n";
  }
  return 0;
}

It's beautiful, isn't it?

thank you for this! in the first line of this code, is it enum class or just enum GroupName {SU, Sp}?

Also, say we manage to get rid of OmegaInvariance, do we still want that is_sp struct?

Once we are settled, and we agree this is our best option, I'll check with Peter