Add perturbation option to MagnetizedTovStar #6016
Conversation
| template <typename DataType, StarRegion Region> | ||
| void MagnetizedTovVariables<DataType, Region>::operator()( | ||
| const gsl::not_null<tnsr::I<DataType, 3>*> spatial_velocity, | ||
| [[maybe_unused]] const gsl::not_null<Cache*> cache, | ||
| hydro::Tags::SpatialVelocity<DataType, 3> /*meta*/) const { | ||
| if constexpr (Region == StarRegion::Center or | ||
| Region == StarRegion::Exterior) { | ||
| get<0>(*spatial_velocity) = 0.; | ||
| get<1>(*spatial_velocity) = 0.; | ||
| get<2>(*spatial_velocity) = 0.; | ||
| } else { | ||
| const auto& areal_radius = | ||
| radial_solution.coordinate_system() == | ||
| RelativisticEuler::Solutions::TovCoordinates::Isotropic | ||
| ? get(cache->get_var( | ||
| *this, | ||
| RelativisticEuler::Solutions::tov_detail::Tags::ArealRadius< | ||
| DataType>{})) | ||
| : radius; | ||
| get<0>(*spatial_velocity) = perturbation * coords.get(0) / areal_radius; | ||
| get<1>(*spatial_velocity) = perturbation * coords.get(1) / areal_radius; | ||
| get<2>(*spatial_velocity) = perturbation * coords.get(2) / areal_radius; | ||
| } | ||
| } |
There was a problem hiding this comment.
Here's where the perturbation is actually applied.
There was a problem hiding this comment.
Looks good! Just a couple of comments. As for tests, I think there's very little we can do, but a run demonstrating that everything is working as expected (i.e. the velocity that comes out can be correctly interpreted in terms of the perturbation that was put in) would go a long way.
| @@ -38,16 +38,19 @@ MagnetizedTovStar::MagnetizedTovStar( | |||
| const RelativisticEuler::Solutions::TovCoordinates coordinate_system, | |||
| std::vector<std::unique_ptr< | |||
| grmhd::AnalyticData::InitialMagneticFields::InitialMagneticField>> | |||
| magnetic_fields) | |||
| magnetic_fields, | |||
| const double perturbation) | |||
There was a problem hiding this comment.
Maybe a more specific name, like radial_velocity_perturbation?
| get<1>(*spatial_velocity) = 0.; | ||
| get<2>(*spatial_velocity) = 0.; | ||
| } else { | ||
| const auto& areal_radius = |
There was a problem hiding this comment.
Hm, I'm a bit worried about the normalization here. I think probably the number that gets passed in should be the norm of the spatial velocity. |v| =sqrt(v^iv^j\gamma_{ij}). This way users would know if the perturbation they're passing in is reasonable or not. In this case this is pretty straightforward, for Schwarzschild coordinares you just have to multiply the spatial velocity by sqrt(\gamma^{rr}) = sqrt(1-2M(r)/r). I think for isotropic coordinates what you have is already correct, because \gamma_rr = \psi^4= (areal_radius/coord_radius)^2, so just dividing by the areal radius is all you have to do to get the spatial velocity to be normalized correctly. You should be able to check this by starting a run, and the max value of the spatial velocity in the domain should be the value you input to the input file.
| @@ -215,7 +215,7 @@ struct TovVariables { | |||
| void operator()(gsl::not_null<Scalar<DataType>*> lorentz_factor, | |||
There was a problem hiding this comment.
I think this also needs to be modified, right now I'm pretty sure it's being used to compute prim2con here , because initialization is just using the primitive variables tag list here, which points to this. I would love to be wrong about this though, because we don't really need the Lorentz factor if we have the spatial velocity and the spatial metric.
|
@shabibti Since it's been a while, if you'd like to have these changes merged into spectre, please comment here or let the core-devs know. If you would no longer like to have these changes merged, or we don't hear from you by next Wed Aug 28 1pm PDT, we will close this PR. If you change your mind in the future, we can always re-open the PR. |
|
@knelli2 I'm planning to edit this a bit once I find the time, so I'd like to keep it open. |
knelli2
added
the
in progress
Proposed changes
Adds a
Perturbationinput option toMagnetizedTovStarfor the size of an initial radially symmetric perturbation applied tospatial_velocityin the interior star region. This is potentially useful for convergence testing/measuring oscillation modes in a high resolution run.Code review checklist
make docto generate the documentation locally intoBUILD_DIR/docs/html.Then open
index.html.code review guide.
bugfixornew featureif appropriate.Further comments
I could use feedback over what tests would be useful to add and whether there's a better formulation to use.