Add 0th order puncture field to CurvedScalarWave #4760
Conversation
nikwit
changed the title
nikwit
force-pushed
the
Add-Puncture-field-0
branch
from
9385e97
to
eae915c
Compare
| evolved_vars->initialize(get<0>(coords).size()); | ||
| const double r0 = orbital_radius; | ||
| const double M = BH_mass; | ||
| const double w = pow(r0, -1.5); |
There was a problem hiding this comment.
not a full review, just a quick note. I realize most of this code probably comes from Mathematica, but assuming you'll be calling this quite frequently during the evolution you should go through and do optimizations like pow(x, 2) -> pow<2>(x), or pow(r0, -1.5)->1.0 / (r0 * sqrt(r0)) (pow is insanely expensive, you can do about 180 mult/add for one pow)
Edit: and also things like 10 -> 10.
nikwit
force-pushed
the
Add-Puncture-field-0
branch
3 times, most recently
from
3131187
to
31ccd27
Compare
| DynamicBuffer<DataVector> temps(66, grid_size); | ||
|
|
||
| const double d_0 = 1.0 / r0; | ||
| const double d_1 = 3 * M; |
There was a problem hiding this comment.
is it easy to regex-replace [space][number][space with [space][number].0[space]? If not, I wouldn't worry about it... This is auto-generated code anyway
There was a problem hiding this comment.
it's not that easy unfortunately because the spaces only appear after I clang-format. Could probably be done with a bit of work but might not be worth the effort - should be the same binary after all.
There was a problem hiding this comment.
nevermind, fixed it now
nikwit
force-pushed
the
Add-Puncture-field-0
branch
from
31ccd27
to
d514be2
Compare
|
thank you, I squashed it in :) |
nikwit
force-pushed
the
Add-Puncture-field-0
branch
from
d514be2
to
fb7212e
Compare
nikwit
force-pushed
the
Add-Puncture-field-0
branch
from
fb7212e
to
e2ad85a
Compare
src/Evolution/Systems/CurvedScalarWave/Worldtube/PunctureField.hpp
Outdated
Show resolved
Hide resolved
src/Evolution/Systems/CurvedScalarWave/Worldtube/PunctureField.hpp
Outdated
Show resolved
Hide resolved
| gsl::not_null<Variables<tmpl::list< | ||
| Tags::Psi, ::Tags::dt<Tags::Psi>, | ||
| ::Tags::deriv<Tags::Psi, tmpl::size_t<3>, Frame::Inertial>>>*> | ||
| evolved_vars, |
There was a problem hiding this comment.
Question @nilsdeppe: do you have any preference these days on passing Variables vs individual Tensors to functions like this? I often found it easier to work exclusively with Tensors in pointwise functions because then the function is agnostic to where the Tensors are actually stored (could be in separate Variables or in one Variables together with other things, or on a Variables with prefixes tags). Now we have nonowning variables so we could create suitable Variables when calling this function. But overall my experience is that avoiding Variables and Tags in pointwise functions makes everything easier. Thoughts?
There was a problem hiding this comment.
A quick reply now, I can elaborate more if you want. I've started adding Variables-based overloads to GRMHD and GH for things like time derivatives. The motivation there being that 1. there are fewer arguments 2. we can now use structured bindings to easily extract out of a Variables 3. non-owning Variables makes it easier to deal with and 4. the DataStructures/TaggedContainer.hpp code makes working with multiple tagged containers simultaneously very easy and so overcomes another hurdle we had with using Variables more.
Basically, I think there are cases in generic code where manipulating a Variables (or set of them) is significantly easier than trying to get the right Tensor from the right tagged container, and so I'm generally in favor of doing so when it make the code a lot easier to read and write. E.g. there were places in GHGRMHD where we would take a bunch of takes from 3 or so tagged containers, put references of them in a TuppleTuple, just so we could pull them out later. This was extremely parameter pack and template dense code that made it hard to read, but the recent additions I listed above make it so that isn't needed anymore.
There was a problem hiding this comment.
Sounds reasonable, I'll give the new tools a try. Maybe generic Variables would also be a way to add new evolution variables like recently the electron fraction without refactoring half the code base.
There was a problem hiding this comment.
That was one of the main motivations for the added functionality.
nikwit
force-pushed
the
Add-Puncture-field-0
branch
from
e2ad85a
to
6600894
Compare
nikwit
force-pushed
the
Add-Puncture-field-0
branch
from
6600894
to
36116f4
Compare
| * \brief Computes the puncture/singular field \f$\Psi^\mathcal{P}\f$ of a | ||
| * scalar charge in circular orbit around a Schwarzschild black hole as | ||
| * described in \cite Detweiler2003 expanded to order 0 in geodesic distance. | ||
| */ |
There was a problem hiding this comment.
Add \see puncture_field to reference the main function if you get the chance (not holding up this PR)
Proposed changes
Adds the Detweiler-Whiting singular/puncture field expanded up to zeroth order in geodesic distance from a scalar charge.
Update: I vectorized the calculation of the puncture field which makes the calculation about 7x faster than before on a single core on my local machine.