sxs-collaboration / spectre

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

Argument types for MathFunctions #462

Open nilsdeppe opened 6 years ago

nilsdeppe commented 6 years ago

Feature request:

MathFunctions currently take DataVectors, which works in 1D but is not what we want in higher dimensions. We need to be able to deal with situations like ScalarWave::Solutions::PlaneWave where there is effective dimension collapsing happening and then the MathFunction is called on the resulting collapsed "coordinates". A discussion with @wthrowe @kidder @gsb76 and myself resulted in the conclusion that the current most promising path forward is for MathFunction to take a tnsr::i<T, Dim, Frame> where we generated explicit instantiations for both Frame::Inertial (the "normal" case where you're passing cartesian coordinates) and Frame::NoFrame where you are passing something like the collapsed-dimension coordinates in ScalarWave::Solutions::PlaneWave.

osheamonn commented 6 years ago

I just want to make sure I understand the plan here. How will this handle the time dependence of PlaneWave? The dimension collapsing as you call it is not from 3d to 1d but is from 3+1d to 1d, since the resulting one dimensional function is called on k.x - wt.

nilsdeppe commented 6 years ago

Yes, the idea is that we still have the function u that computes the 1D "coordinate". That is then passed as a tnsr::I<DataVector, 1, Frame::NoFrame> to the MathFunction<1>. Does that help?

osheamonn commented 6 years ago

I suppose I'm just not seeing the advantage, and I would find it conceptually surprising that the function 'x -> sin(kx)' would take a 1d tensor of doubles/DataVectors as an argument instead of a double/DataVector, or even a Scalar.

I thought that the original plan was for 1d mathfunctions to be a partial specialisation that takes DataVectors/doubles, so there'd be no need for specifying NoFrame, and for the general dimension case to take tensors which would be Frame::Inertial (or assert that the frame is physical or whatever). It seems in 2d and 3d you shouldn't even be evaluating the function unless the frame is physical so NoFrame should be disallowed in those cases.

nilsdeppe commented 6 years ago

Well why should the interface to 1d be different from 2d and 3d? Having a different interface is a major disadvantage, especially for generic code. If we have a different interface we have to specialize all generic code on dimension. Now what's not clear is how much of this generic code we will have, but I'd be surprised if the answer is none.

In terms of allowing NoFrame in 2D and 3D, why could we have a mapping from 2/3 + 1d to 2/3d? Given that the difference between allowing and disallowing is a single argument to the GENERATE_INSTANTIATIONS macro I think that NoFrame instantiations can easily be delayed to later on when we know we want them for 2 and 3d. I think it would be easiest in the long run to write the classes as being templated on Frame and just restrict instantiations.

wthrowe commented 6 years ago

The 1D case could be specialized to take either.

nilsdeppe commented 6 years ago

That sounds fine to me

geoffrey4444 commented 4 years ago

Addressed in #2300