Add m1 analytic data & couple GreyM1 to fixed hydro #6453

pajkosmi · 2025-01-24T03:21:50Z

Proposed changes

This PR adds the homogeneous sphere GreyM1 test problem, couples the fluid to a fixed hydro background, and allows the user to specify the M1Grey initial data in the yaml file.

Upgrade instructions

Code review checklist

The code is documented and the documentation renders correctly. Run
make doc to generate the documentation locally into BUILD_DIR/docs/html.
Then open index.html.
The code follows the stylistic and code quality guidelines listed in the
code review guide.
The PR lists upgrade instructions and is labeled bugfix or
new feature if appropriate.

Further comments

wthrowe

Haven't looked through the entirety of the second commit yet, because it has 1600+ lines of changes. It looks like it's doing several things (updating the executable, reorganizing the boundary conditions, adding new analytic data). Are they separate enough that they could be split into separate commits, at least for review purposes?

wthrowe · 2025-01-27T22:37:33Z

src/Evolution/Systems/RadiationTransport/M1Grey/Sources.cpp

@@ -11,6 +11,7 @@
 #include "DataStructures/Tensor/EagerMath/RaiseOrLowerIndex.hpp"
 #include "DataStructures/Tensor/Tensor.hpp"
 #include "DataStructures/Variables.hpp"
+#include "M1HydroCoupling.hpp"


wthrowe · 2025-01-27T22:38:08Z

src/Evolution/Systems/RadiationTransport/M1Grey/Sources.cpp

+    const Scalar<DataVector>& lorentz,
+    const Scalar<DataVector>& sqrt_det_spatial_metric
+
+) {


Formatting.

wthrowe · 2025-01-27T22:39:43Z

src/Evolution/Systems/RadiationTransport/M1Grey/TimeDerivativeTerms.hpp

+      const Scalar<DataVector>& lorentz,
+      const Scalar<DataVector>& sqrt_det_spatial_metric
+
+  ) {


Formatting.

wthrowe · 2025-01-27T22:45:51Z

src/Evolution/Executables/RadiationTransport/M1Grey/EvolveM1Grey.hpp

        tmpl::pair<evolution::initial_data::InitialData, initial_data_list>,
+        tmpl::pair<ImexTimeStepper, TimeSteppers::imex_time_steppers>,
+


Fixing the alphabetization is good, but we generally use POSIX sorting where all lowercase letters are after all uppercase, so this should put evolution::initial_data::InitialData at the end.

wthrowe · 2025-01-27T22:50:02Z

src/Evolution/Systems/RadiationTransport/M1Grey/TimeDerivativeTerms.hpp

@@ -108,6 +108,7 @@ struct TimeDerivativeTerms {
        extrinsic_curvature, spatial_metric, emissivity, absorption_opacity,
        scattering_opacity, tilde_j, tilde_h_normal, tilde_h_spatial,
        spatial_velocity, lorentz, sqrt_det_spatial_metric));
+    return {true};


Squash into appropriate commit.

pajkosmi · 2025-01-28T23:13:11Z

Haven't looked through the entirety of the second commit yet, because it has 1600+ lines of changes. It looks like it's doing several things (updating the executable, reorganizing the boundary conditions, adding new analytic data). Are they separate enough that they could be split into separate commits, at least for review purposes?

I'll do some more reorganizing later this week. Are you ok if I split into commits that don't independently compile, and then squash them together at the end of review?

wthrowe · 2025-01-28T23:18:16Z

Yes, if it's too interconnected the split can be just during the review.

pajkosmi · 2025-01-29T23:24:24Z

ping, I have reorganized the commits.

wthrowe

Thank you, much less daunting split up like that.

wthrowe · 2025-01-30T22:18:15Z

src/PointwiseFunctions/AnalyticData/RadiationTransport/M1Grey/HomogeneousSphere.cpp

+  const double sharpness = -0.03;
+  return Scalar<DataVector>{
+      (inner_value - outer_value) / M_PI *
+          atan((sqrt(square(x)) - sphere_radius) / sharpness) +


sqrt(square(x)) -> x (this is only called on non-negative values).

wthrowe · 2025-01-30T22:20:20Z

src/PointwiseFunctions/AnalyticData/RadiationTransport/M1Grey/HomogeneousSphere.cpp

+                                         const double outer_value,
+                                         const double sphere_radius) {
+  // the closer to 0 this becomes, the sharper the discontinuity
+  const double sharpness = -0.03;


Is it worth making this a parameter?

wthrowe · 2025-01-30T22:22:55Z

src/PointwiseFunctions/AnalyticData/RadiationTransport/M1Grey/HomogeneousSphere.cpp

+#undef EBIN
+#undef GENERATE_LIST
+
+// template class HomogeneousSphere;


wthrowe · 2025-01-30T22:23:35Z

src/PointwiseFunctions/AnalyticData/RadiationTransport/M1Grey/HomogeneousSphere.cpp

+          tmpl::list<TAG(data) < Frame::Inertial, NTYPE(data) < EBIN(data)> >> \
+          /*meta*/) const;
+
+#define temp_list \


TEMP_LIST for macros (in a few places)

wthrowe · 2025-01-30T22:28:05Z

src/PointwiseFunctions/AnalyticData/RadiationTransport/M1Grey/HomogeneousSphere.hpp

+  static constexpr Options::String help = {
+      "A homogeneous sphere emitting and absorbing neutrinos."};
+
+  // The sphere radius.


// -> ///

wthrowe · 2025-01-30T22:46:28Z

tests/Unit/PointwiseFunctions/AnalyticData/RadiationTransport/M1Grey/Test_HomogeneousSphere.cpp

+
+  // All components of momentum density should be zero for this problem
+  CHECK(min(get<0>(tilde_s)) == 0.0);
+  CHECK(min(get<0>(tilde_s)) == max(get<0>(tilde_s)));


I believe get<0>(tilde_s) == 0.0 works. Similar in a few other places.

wthrowe · 2025-01-30T22:48:51Z

tests/Unit/PointwiseFunctions/AnalyticSolutions/RadiationTransport/M1Grey/TestFunctions.py

@@ -26,3 +26,31 @@ def constant_m1_tildeS(x, t, mean_velocity, comoving_energy_density):


 # End Functions for testing ConstantM1.cpp
+
+
+def homogen_sphere_m1_tildeE(


The C++ part of this comparison test is missing. These should also be moved from AnalyticSolutions to AnalyticData.

wthrowe · 2025-01-30T22:57:09Z

src/Evolution/Systems/RadiationTransport/M1Grey/BoundaryConditions/DirichletAnalytic.cpp

+#include "Evolution/Systems/GrMhd/ValenciaDivClean/Tags.hpp"
+#include "Evolution/TypeTraits.hpp"
+#include "NumericalAlgorithms/Spectral/Mesh.hpp"
+#include "Options/String.hpp"


Check the includes. Some (like this one) seem suspicious.

wthrowe · 2025-01-30T23:01:06Z

src/Evolution/Systems/RadiationTransport/M1Grey/BoundaryConditions/DirichletAnalytic.cpp

@@ -0,0 +1,242 @@
+// Distributed under the MIT License.


This is just moving things into a cpp file without functionality changes, correct?

wthrowe · 2025-01-30T23:13:30Z

src/Evolution/Systems/RadiationTransport/M1Grey/Initialize.hpp

+              initial_data_evo_tags{});
+        });
+
+    const auto initial_data_hydro_vars = call_with_dynamic_type<


Combine the two calls to call_with_dynamic_type, just because I suspect it may be expensive to compile. You can return a pair and use structured bindings to keep the variables as they are now.

pajkosmi added the new feature Adds a new feature that's worth highlighting in release notes label Jan 24, 2025

pajkosmi marked this pull request as ready for review January 24, 2025 17:59

pajkosmi requested a review from wthrowe January 24, 2025 17:59

pajkosmi changed the title ~~Add m1 analytic data & couple hydro to fixed hydro~~ Add m1 analytic data & couple GreyM1 to fixed hydro Jan 24, 2025

wthrowe requested changes Jan 27, 2025

View reviewed changes

pajkosmi force-pushed the add_M1_analytic_data branch 2 times, most recently from 7c24d0a to 3ba9bdd Compare January 29, 2025 23:08

pajkosmi added 6 commits January 29, 2025 15:11

Add homogeneous sphere analytic data

e46774f

Update constantM1 analytic solution

e1dae73

Update M1Grey dirichlet analytic

c2e5eca

CoupleM1Grey to fixed hydro background

26e24ab

Add general M1Grey AnalyticData files

52a27f5

general AnalyticSolution and AnalyticData changes to M1Grey system

6cae38e

pajkosmi force-pushed the add_M1_analytic_data branch from 3ba9bdd to 6cae38e Compare January 29, 2025 23:22

wthrowe requested changes Jan 30, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add m1 analytic data & couple GreyM1 to fixed hydro #6453

Add m1 analytic data & couple GreyM1 to fixed hydro #6453

pajkosmi commented Jan 24, 2025

wthrowe left a comment

wthrowe Jan 27, 2025

wthrowe Jan 27, 2025

wthrowe Jan 27, 2025

wthrowe Jan 27, 2025

wthrowe Jan 27, 2025

pajkosmi commented Jan 28, 2025

wthrowe commented Jan 28, 2025

pajkosmi commented Jan 29, 2025

wthrowe left a comment

wthrowe Jan 30, 2025

wthrowe Jan 30, 2025

wthrowe Jan 30, 2025

wthrowe Jan 30, 2025

wthrowe Jan 30, 2025

wthrowe Jan 30, 2025

wthrowe Jan 30, 2025

wthrowe Jan 30, 2025

wthrowe Jan 30, 2025

wthrowe Jan 30, 2025

		tmpl::pair<evolution::initial_data::InitialData, initial_data_list>,
		tmpl::pair<ImexTimeStepper, TimeSteppers::imex_time_steppers>,

		@@ -26,3 +26,31 @@ def constant_m1_tildeS(x, t, mean_velocity, comoving_energy_density):


		# End Functions for testing ConstantM1.cpp


		def homogen_sphere_m1_tildeE(

Add m1 analytic data & couple GreyM1 to fixed hydro #6453

Are you sure you want to change the base?

Add m1 analytic data & couple GreyM1 to fixed hydro #6453

Conversation

pajkosmi commented Jan 24, 2025

Proposed changes

Upgrade instructions

Code review checklist

Further comments

wthrowe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pajkosmi commented Jan 28, 2025

wthrowe commented Jan 28, 2025

pajkosmi commented Jan 29, 2025

wthrowe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment