sxs-collaboration / spectre

SpECTRE is a code for multi-scale, multi-physics problems in astrophysics and gravitational physics.
https://spectre-code.org
Other
161 stars 191 forks source link

Remove mixed valence dot_product? #583

Open wthrowe opened 6 years ago

wthrowe commented 6 years ago

I think we screwed up in adding the mixed-valence dot_product overload in #572. It breaks our tensor type safety, in that if you are mistaken about the valence of a vector the code will compile and do a (perhaps subtly) wrong thing. For example, this looks like a tensor contraction but is actually performing a non-covariant operation because you forgot we use a lower-index face normal:

tnsr::i<DataVector, volume_dim> my_covector(...);
auto n_dot_v = dot_product(my_covector, db::get<Tags::UnnormalizedFaceNormal<volume_dim>>(box));

If dot_product only worked on same-valence vectors this would still compile, but it would be clear from looking at the code that it was doing a Euclidean dot product rather than the intended contraction.

wthrowe commented 6 years ago

After conversations with several people, it looks like the preferred direction to go here is to remove the same valence dot_product overload (keeping the metric version) and instead add a Euclidean index type for Tensor that would be contractible with any index type. Most code that takes tensors as arguments (outside of systems, which should know whether they are Euclidean) is generic over the index valence, so this should not be too intrusive in existing code.

wthrowe commented 6 years ago

Some points from today's discussion (centered around #609):

I've probably forgotten something, since I didn't think to make notes when it was fresh in my mind.

Since we determined that Euclideanness is really a property of a frame, I would next like to experiment with encoding the information in the frame type used in tensors (which would in practice probably be specified by the system, since the system implementation will assume either Euclidean or not).

nilsdeppe commented 6 years ago

Depends on results of #609

wthrowe commented 6 years ago

I've tried experimenting with encoding Euclideanness in frames. It seems more reasonable than the attempt with indices, but it requires more changes so I haven't gotten the entire code working with it. The difficulties I've found are: