Added Fastor as a math backend to detray #963
Conversation
|
|
Hi, Unfortunately I don't think we will move away from the single precision literals any time soon. Double precision operations can be costly on some GPUs (and are less efficient to vectorize) and we want to prevent any implicit (unwanted) upcasting from double precision literals to avoid them in single precision builds. Therefore, the compilers in the CI warn very strictly on type conversions. The fun with expression templates we have also seen in other plugins (mostly Eigen and SMatrix). Normally, using explicit types/casts in the plugin or in detray should be enough to fix this. In the Eigen plugin, for example, the functions take a type that can be used both with the vector types and the operation types (but I do not know if that is possible in Fatsor, potentially using the |
That makes sense. I'll revert that commit and try to debug what the issue with Fastor's template type deduction is.
I was looking into it, and I think you are correct. I will need to make some changes to the Fastor backend in The issue is not with Fastor, but with some of the function definitions in the Fastor backend for In particular, this code works as intended: #include <iostream>
#include "Fastor/Fastor.h"
int main() {
Fastor::Tensor<double, 3> v1({1., 2., 3.});
Fastor::Tensor<double, 3> v2({4., 5., 7.});
double ans = Fastor::inner(v1, v1 + v2);
std::cout << "v1 =\n" << v1 << "\tv2 =\n" << v2 << '\n';
std::cout << ans << '\n';
} |
This reverts commit 3d28674.
|
2 participants
This PR is a big one because of certain quirks in the Fastor library.
To ease the reviewing process, I will explain the changes that were made and why they were made.
Large Number of Files Changed
The majority of the changes are files where I removed the
fsuffix in floating point numbers.In
core/include/detray/builders/cuboid_portal_generator.hpp, we had the line (before I changed it)This causes problems in Fastor because the compiler tries to instantiate a
Fastor::Tensor<double, 3>, but the braced-initializer-list contains 3floats. Because of this, the template deduction was failing and the compilation would error out.Logically also, we are typecasting from a smaller type (
float) to a larger type (double), so it makes sense to have the default be adoublethat might be downcasted to afloatif that is what we define ourscalar_tto be.This was occurring in many places, so I changed it using a search-and-replace. In certain cases, I made changes to files which were using
floats, and I triggered the-Wfloat-conversionwarning, so I reverted the change in those files. After reverting those files, I wasn't getting any more warnings of that type, so I have some amount of assurance that the changes weren't too heavy-handed.However, I still worry that I changed some files that didn't need to be changed. If anyone has any ideas on how to determine if each file that had the
fsuffix change actually needs it, I'm open to suggestions.Fastor Expression Template Shenanigans
In some places, I had to special=case the math computations for the Fastor library specifically using
#ifdefbecause otherwise, the compilation would fail.First Example
There is this line:
in
core/include/detray/geometry/shapes/annulus2D.hpp.When the Fastor backend is used, both
candbare of typepoint_t, which areFastor::Tensor<T, 2>(Tis eitherfloatordouble). When operations betweenTensors are performed, they're not done eagerly, but lazily to prevent extra computations. In order to achieve this,c + breturns aFastor::BinaryAddOp.The issue here is that
normalizedoesn't have an overload that takes in aFastor::BinaryAddOpas input. For this reason, it throws a compilation error. The easiest way around this was to use a temporary variabletmpwith an explicit type to force the computation to happen.Second Example
There is this line:
in
core/include/detray/geometry/coordinates/line2D.hpp.The behavior here is very similar to the previous case.
t - preturns aFastor::BinarySubOp, which thedotfunction doesn't know how to handle, so it causes a compilation error with the Fastor backend. Here also, using a temporary variable fixes the issue.Performing operations like dot products, cross products, normalization, is by all means a very common usecase, so we should not require such workarounds. But until we get upstream fixes in Fastor, this is the best workaround I can think of.