Error stepper action reorder #2862

moxcodes · 2021-02-10T00:38:18Z

Proposed changes

This factors time stepper routines into free functions so that they can be performed as a subroutine of ComputeTimeDerivative, and perform the alteration of the order of operations in the local-time-stepping case to support step unwinding -- to be able to reject and re-try a step based on information from a step chooser, the step chooser must run after the time-stepper update_u and before the component sends flux information to the neighbors, so the entire loop must be placed as a subroutine of ComputeTimeDerivative .

Upgrade instructions

Be sure to update any local time-stepping action lists to match the control flow in the EvolveScalarWave.hpp in this PR.
Note that when using local time-stepping and not in self-start, the ComputeTimeDerivative action now performs the steps previously done by RecordTimeStepperData, UpdateU, and ChangeStepSize.
Those actions are still available for cases in which either local time-stepping is not used or ComputeTimeDerivative is not used.

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
major new feature if appropriate.

nilsdeppe · 2021-02-10T00:58:17Z

src/Evolution/DiscontinuousGalerkin/Actions/ComputeTimeDerivative.hpp

  }

+  if constexpr (Metavariables::local_time_stepping and
+                not SelfStart::is_self_start_procedure<ActionList>) {


I don't love the is_self_start_pro... because subcell will likely wreak a lot of havoc in self starting (this was the case in my previous implementation where I never got it far enough to actually fully work, though I'm hopeful I can simplify it a lot this time around). Couldn't this check be done via runtime information from either the time stepper and/or the slab? E.g. I thought all slabs during self-start are before (in the sense of the direction that time flows) the initial time. That seems a lot less likely to break and causes less subtle coupling between pieces of code that might check if they are in self-start. I also don't think you can do LTS until a certain amount of time after self-start because the history is non-monotonic, but @wthrowe will need to say what the restrictions are there.

(I haven't looked at the rest of the code, I just wanted to see the changes in ComputeTimeDerivative)

The sanctioned method of determining you are in self-start is checking if the slab number is negative, but I'm not sure why you want to conditionally take a step anyway.

Here, the point is that we can only make the stepping a subroutine of the ComputeTimeDerivative during the main evolution and not during self-start, because self-start needs to replace the RecordTimeStepperData with it's own looping mechanism. During self-start, the action list explicitly contains the UpdateU and RecordTimeStepperData actions.

I also don't like that this adds another level of nested conditionals into the Metavariables. I'd generally prefer going the other direction. Maybe we can discuss this at the call today (Wed. Feb. 10) if both @wthrowe and @moxcodes can make it. DG-subcell needs even more jumping around in the action list (roughly), and making the action list conditional on self-starting will probably mean I have to rip out the error adjustment to get DG-subcell to work.

wthrowe

I've only skimmed most of this. Please explain what you are trying to do.

wthrowe · 2021-02-10T01:48:51Z

src/Evolution/Executables/Burgers/EvolveBurgers.hpp

+                      step_actions,
                      tmpl::conditional_t<
                          local_time_stepping,
                          Actions::ChangeStepSize<step_choosers>, tmpl::list<>>,
-                      step_actions, Actions::AdvanceTime>>>>>;
+                      Actions::AdvanceTime>>>>>;


Doesn't this break the communication of the step size?

wthrowe · 2021-02-10T02:02:03Z

src/Time/TimeSteppers/TimeStepper.hpp

+  /// Whether a change in the slab size is permitted -- this is to be evaluated
+  /// immediately after the time step id has been advanced.
+  template <typename Vars, typename DerivVars>
+  bool can_change_slab_size(


You should never need to forbid a slab size change (except during self-start, but that's special). What are you trying to do here? The current behavior is required for observations without dense output.

Here I'm just trying to preserve the current behavior in ChangeSlabSize -- because in my suggested strategy the ChangeStepSize must now happen after the step is evaluated by update_u (and the slab is still changed before the update), the time step id is now different for the case when the step can change as compared to when the slab can change.

edit: I suppose that, because the determination in ChangeSlabSize is exclusively for determining whether or not we're in self-start, I could entirely remove the can_change_slab_size in favor of a runtime function independent of stepper that determines whether the time steps are ordered (so, sufficiently far after self-start).

wthrowe · 2021-02-10T02:07:42Z

src/Time/Actions/ChangeSlabSize.hpp

+      // matches the previous step value
+      const auto& step_controller = db::get<Tags::StepController>(box);
+      new_step = step_controller.choose_step(new_time_step_id.step_time(),
+                                             current_step.value());


This is susceptible to roundoff error causing step-size fluctuations. I suggest preventing the slab size from increasing rapidly instead of trying to deal with the fallout when it does. (There is already code to do this: add the "Increase" StepChooser to your input file.)

wthrowe · 2021-02-10T02:11:12Z

src/Time/TakeStep.hpp

+          typename DbTags, typename Metavariables>
+void take_step(const gsl::not_null<db::DataBox<DbTags>*> box,
+               const Parallel::GlobalCache<Metavariables>& cache,
+               const VariablesTag /*meta*/ = VariablesTag{}) noexcept {


Why do you need this as an argument? Doesn't the template parameter work on its own?

You're right -- it's a better interface to just always specify the template argument, I'll omit the extra argument.

wthrowe · 2021-02-10T02:18:01Z

src/Evolution/DiscontinuousGalerkin/Actions/ComputeTimeDerivative.hpp

  }

+  if constexpr (Metavariables::local_time_stepping and
+                not SelfStart::is_self_start_procedure<ActionList>) {


The sanctioned method of determining you are in self-start is checking if the slab number is negative, but I'm not sure why you want to conditionally take a step anyway.

moxcodes · 2021-02-10T04:13:34Z

So the main goal here is to get the step change to be between the update u and the advance time.
During an accepted step, that means the step being chosen isn't Tags::TimeStep, but a new tag I've added Tags::Next<Tags::TimeStep>, so when advancing the step, it uses the current step size, then assigns the step determined by Tags::Next<Tags::TimeStep> to the current step before proceeding.
During a rejected step, it will need to modify the current step as well as asking for a different next step.

Perhaps I have made the changes more confusing than they need to be in the process of trying to chunk them out into reasonably intuitive commits:

in the earlier commit Reorder step change to before AdvanceTime, I make the change associated with determining the Tags::Next<Tags::TimeStep>, and performing an initial reorder of the actions. This is an intermediate state that preserves existing functionality, but wouldn't be usable for step unwinding
in the later commit Use take_step in ComputeTimeDerivative, in the case of local time stepping and when not in the self start procedure, the step change is performed as a subroutine of ComputeTimeDerivative (so at yet a different stage in the process), where I actually need it to be to support step unwinding.

Does that make my intent more clear? Or is there something else I'm missing that still seems poorly motivated?

moxcodes · 2021-03-09T18:59:01Z

This has been updated now that the other time-stepper utilities have been merged. It no longer makes a distinction between the self-start procedure and the main evolution.
Cheers!

wthrowe · 2021-03-09T21:25:15Z

src/Time/StepChoosers/Cfl.hpp

+    // Reject the step if the CFL condition is violated. In the case of CFL
+    // violation, ensure that the step diminishes slightly, to avoid the
+    // possibility of roundoff-scale fluctuations causing a retry with the same
+    // rejected step-size.


I don't understand this. The rejection condition is step_size < last_step_magnitude, so how could there be a retry with the same last_step_magnitude?

wthrowe · 2021-03-09T21:33:39Z

src/Time/Actions/ChangeStepSize.hpp

-                         : std::numeric_limits<double>::infinity();
+  const double last_step_size = history.size() > 0
+                                    ? db::get<Tags::TimeStep>(*box).value()
+                                    : std::numeric_limits<double>::infinity();


I think it's reasonable to always use db::get<Tags::TimeStep>(*box).value(), unless it might not be set to something sane on the first step.

wthrowe · 2021-03-09T21:44:03Z

src/Time/TimeSteppers/AdamsBashforthN.hpp

+  const evolution_less_equal<Time> less_equal{time_id.time_runs_forward()};
  return history.size() == 0 or
-         (less(history.back(), time_id.step_time()) and
+         (less_equal(history.back(), time_id.step_time()) and


What is this change about?

In the case of a length 1 history, the step size change should be permitted when the time step id is the same as the last in the history, because the step change now occurs before the advance time. The alternative, I suppose, would be to keep the same comparison operator but check the next time id instead of the current time id.

This check is supposed to determine the "local direction of time flow". If the two times are equal then something is wrong. The goal of the check is to reject the case where, e.g., the previous steps were at times 1, 2, 3 and the next step will be at 0 (because the is_sorted check will pass there).

I see. I think the way to keep this condition consistent with previous cases is then to just pass in the next time step (Tags::Next<Tags::TimeStepId>) instead of the current one (Tags::TimeStepId) and then I can revert the changes to the contents of the check.

wthrowe · 2021-03-09T21:46:50Z

tests/Unit/Helpers/Time/TimeSteppers/TimeStepperTestUtils.cpp

  id = stepper.next_time_id(id, slab.duration() / 2);
  if (id.substep() != 0) {
    history.insert(id, 0.0, 0.0);
-    CHECK(not stepper.can_change_step_size(id, history));


What is this change about? This is testing the behavior of substep integrators, but you're not changing any of the substep integrators.

wthrowe · 2021-03-09T22:03:44Z

tests/Unit/Time/Actions/Test_SelfStartActions.cpp

    const bool forward_in_time, const Time& initial_time,
-    const TimeDelta& initial_time_step, const size_t order,
-    const double initial_value) noexcept {
+    const TimeDelta& initial_time_step, const TimeDelta& initial_next_time_step,


Does it ever make sense for these to be different?

wthrowe · 2021-03-09T22:06:22Z

src/Time/TakeStep.hpp

+          typename DbTags, typename Metavariables>
+void take_step(const gsl::not_null<db::DataBox<DbTags>*> box,
+               const Parallel::GlobalCache<Metavariables>& cache,
+               const VariablesTag /*meta*/ = VariablesTag{}) noexcept {


wthrowe · 2021-03-10T19:31:47Z

src/Evolution/DiscontinuousGalerkin/Actions/ComputeTimeDerivative.hpp

-      tmpl::list<::dg::Tags::Formulation, evolution::Tags::BoundaryCorrection<
-                                              typename Metavariables::system>>,
-      tmpl::list<::dg::Tags::Formulation>>;
+    tmpl::list<evolution::dg::Tags::BoundaryCorrectionAndGhostCellsInbox<


Something went wrong with the formatting here. I don't know what the "correct" format is, but this line and the previous should line up.

wthrowe · 2021-03-10T19:34:54Z

src/Evolution/Executables/Burgers/EvolveBurgers.hpp

+      tmpl::conditional_t<
+          local_time_stepping,
+          tmpl::list<Actions::MutateApply<boundary_scheme>>,
+          tmpl::list<Actions::MutateApply<boundary_scheme>,


Move the common action outside the conditional. Also for all the other executables.

wthrowe · 2021-03-10T20:27:31Z

tests/Unit/Time/Test_TakeStep.cpp

+  const TimeDelta time_step = slab.duration() / 2;
+  std::unique_ptr<TimeSequence<double>> times{
+      std::make_unique<TimeSequences::Specified<double>>(
+          std::vector<double>{0.005, 0.0075, 0.00875, 0.01})};


I don't understand why this test passes. I think the floating point error in these values should cause the step to drop compared to the analytic value. (Because 0.0075 - 0.005 < 0.01 / 4)

moxcodes · 2021-03-11T21:05:33Z

fixups posted.
Cheers!

wthrowe

The fixups look fine, but I still don't understand the test.

wthrowe · 2021-03-12T01:44:36Z

tests/Unit/Time/Test_TakeStep.cpp

+  take_step<step_chooser_list>(make_not_null(&box), cache);
+  // check that the state is as expected
+  CHECK(db::get<Tags::TimeStepId>(box).substep_time().value() == 0.005);
+  CHECK(db::get<Tags::Next<Tags::TimeStep>>(box) == TimeDelta{slab, {1, 8}});


I don't understand why this is the outcome. We've just taken a 1/4 step from 0.005 to 0.0075, and I think this line means we will next take a 1/8 step, which will put us at 0.00875. But one of the goals was 0.00751, which we've then missed.

moxcodes · 2021-03-12T06:25:23Z

Thanks for pointing that out, @wthrowe -- Yes, it turns out the StepToTimes chooser was actually subtly broken by the reordering. I've fixed it to examine the next time instead, so now it functions as intended.
I've also fixed the test so that the target times are exactly representable as floating-point values so that the StepToTimes chooser doesn't have roundoff-level troubles (added as a new fixup commit).
I've squashed the previous fixups.
Cheers!

moxcodes · 2021-03-12T16:57:46Z

ach, nevermind, now it's broken for slabs...

moxcodes · 2021-03-12T18:37:38Z

Okay, I've added a warning to StepToTimes indicating that it shouldn't be used to choose steps in an LTS scheme (I think because of the roundoff difficulties, this suggestion doesn't restrict any reasonable use-cases), and updated the test to use the Increase step chooser instead.
Cheers!

wthrowe

The test looks good now. Go ahead and squash.

wthrowe · 2021-03-12T19:34:19Z

src/Time/StepChoosers/StepToTimes.hpp

 /// changing immediately is inefficient, it may be best to use
 /// triggers to only activate this check near (within a few slabs of)
 /// the desired time.
+/// \warning This step chooser should be used only to choose slabs, not steps in


Good warning. We generally prevent this in the metavariables, but having the reason in the documentation is good.

wthrowe · 2021-03-12T19:35:07Z

tests/InputFiles/Burgers/Step.yaml


 # Executable: EvolveBurgersStep
 # Check: parse;execute
-


Drop change.

moxcodes · 2021-03-12T22:40:19Z

squashed (and erroneous change dropped).
Cheers!

wthrowe · 2021-03-12T22:41:53Z

tests/Unit/Time/Test_TakeStep.cpp

+  CHECK(db::get<Tags::Next<Tags::TimeStep>>(box) == TimeDelta{slab, {1, 2}});
+  CHECK(db::get<EvolvedVariable>(box) == initial_values * exp(0.005));
+  CHECK(db::get<Tags::dt<EvolvedVariable>>(box) ==
+        1.0e-2 * initial_values * exp(0.0025));


CHECK_ITERABLE_APPROX to avoid trouble with #2984.

moxcodes · 2021-03-12T22:47:40Z

Iterable approx squashed in.
Cheers!

nilsdeppe

Another that I wasn't sure where to comment on that wasn't clear: is the CFL chooser now choosing based only on the volume step? Normally the CFL condition depends on the char speeds at t^n, and here it looks like it's chosen based on the volume vars at t^{n+1}. I don't know how much this matters, or if my understanding of the code is correct.

nilsdeppe · 2021-03-15T18:13:41Z

src/Time/Actions/UpdateU.hpp

 // IWYU pragma: no_forward_declare db::DataBox
 /// \endcond

+/// Perform variable updates for one substep


should this be (sub)step? Since I guess for LMM steppers it updates for one step?

nilsdeppe · 2021-03-15T18:19:49Z

src/Time/Actions/ChangeStepSize.hpp

-  const auto new_step =
-      step_controller.choose_step(time_id.step_time(), desired_step);
+  const auto new_step = step_controller.choose_step(
+      db::get<Tags::Next<Tags::TimeStepId>>(*box).step_time(), desired_step);


why not use the next_time_id variable you defined on line 47 above?

nilsdeppe · 2021-03-15T18:21:31Z

src/Time/Actions/SelfStartActions.hpp

-      detail::vars_to_save<System>, ::Tags::TimeStep>>::simple_tags;
+  using simple_tags = typename StoreInitialValues<
+      tmpl::push_back<detail::vars_to_save<System>, ::Tags::TimeStep,
+                      ::Tags::Next<::Tags::TimeStep>>>::simple_tags;


Could you update the doxygen that now Next<TimeStep> is also saved and why it's saved?

nilsdeppe · 2021-03-15T18:23:25Z

src/Time/CMakeLists.txt

  HEADERS
  BoundaryHistory.hpp
  EvolutionOrdering.hpp
  History.hpp


Commit message "use in outside actions" remove in or something. I'm having trouble parsing this because it makes "outside Actions" sound like a noun, and I don't know what those are

nilsdeppe · 2021-03-15T18:27:16Z

src/Time/TakeStep.hpp

+/// updating the evolved variables and step size.
+///
+/// This function is used to encapsulate any needed logic for updating the
+/// system, and in the case for which step parameters may need to be rejected


I don't understand where the step unwinding is happening. I was expecting a loop inside the take_step function surrounding the update_u and change_step_size calls, but that appears to be absent. What am I missing?

Ah, yes, good catch -- I hadn't added the loop yet because in the initial version, none of the steppers were able to reject a step, but now CFL can, so this should be updated.

moxcodes · 2021-03-15T18:42:47Z

Regarding the CFL condition -- strictly it's now actually checked twice. That might be a somewhat stronger condition than necessary, I'm not certain whether the 'before and after' condition should be any more reliable, but it does certainly prevent drama at slab changes.
The first time it's used to set the 'next step', then after that step is taken, the n+1 step CFL is computed, and if it is smaller than the step actually taken, the step is rejected (or will be once I make the changes from your other suggestion), and re-evaluated. You might anticipate that this would cause a bunch of extra rejections from slight fluctuations in the grid spacing, but because the step controller coarse-grains things by a fair amount, it should cause a rejection only when the grid spacing changes enough to trip over e.g. a factor of 2 scale.

nilsdeppe · 2021-03-15T21:02:11Z

Okay, that sounds reasonable. We can adjust later if necessary, but that sounds like a good way to start. Thanks!

moxcodes · 2021-03-16T00:28:02Z

Posted fixups as well as an additional commit Allow step rejection for LTS schemes that puts in the looping behavior.
Cheers!

nilsdeppe

LGTM, one dox request you can squash in immediately :)

nilsdeppe · 2021-03-16T01:00:58Z

src/Time/Actions/ChangeStepSize.hpp

 template <typename StepChooserRegistrars, typename DbTags,
          typename Metavariables>
-void change_step_size(
+bool change_step_size(


while squashing, could you add to the dox what the returned bool means?

This permits the stepper procedure to be called as a subroutine of other actions or functions

It's useful for testing other time utilities that need a prepared history for AdamsBashforthN

For ease of use in actions which need to invoke the stepper as a subroutine.

This is needed to support step-unwinding as a subroutine of `ComputeTimeDerivative`. The ordering of the time-stepper process ensures that the step unwinding must happen after the volume time-derivative terms are computed, but before the fluxes are sent to neighbors, so must happen as a subroutine of `ComputeTimeDerivative`.

moxcodes · 2021-03-16T04:57:48Z

Squashed.
Cheers!

wthrowe

Squash directly.

wthrowe · 2021-03-16T19:45:16Z

src/Time/Actions/ChangeStepSize.hpp

+                        tmpl::index_if<
+                            ActionList,
+                            tt::is_a_lambda<Actions::UpdateU, tmpl::_1>>,
+                        tmpl::no_such_type_>) {


tmpl::none<ActionList, tt::is_a_lambda<Actions::UpdateU, tmpl::_1>> or store the result of the index_if and use it again below.

moxcodes · 2021-03-18T06:13:17Z

Squashed in the suggested change.
I also noticed a small piece that was flawed logic in the context of step rejection -- the diff for the additional change is:

--- a/src/Time/Actions/ChangeStepSize.hpp
+++ b/src/Time/Actions/ChangeStepSize.hpp
@@ -74,12 +74,10 @@ bool change_step_size(
 
   const auto new_step =
       step_controller.choose_step(next_time_id.step_time(), desired_step);
-  if (new_step != current_step) {
-    db::mutate<Tags::Next<Tags::TimeStep>>(
-        box, [&new_step](const gsl::not_null<TimeDelta*> next_step) noexcept {
-          *next_step = new_step;
-        });
-  }
+  db::mutate<Tags::Next<Tags::TimeStep>>(
+      box, [&new_step](const gsl::not_null<TimeDelta*> next_step) noexcept {
+        *next_step = new_step;
+      });

Cheers!

wthrowe · 2021-03-18T18:40:21Z

src/Time/Actions/ChangeStepSize.hpp

+                                                           tmpl::_1>>::value) {
+        ERROR(
+            "Step not successful, and there is no UpdateU action to return "
+            "to.");


One last question for my understanding: What is this case supposed to handle? If the run is going to die because the chosen step size is a bit off, should it at least give some information about what is wrong? Do we really want to abort in this case instead of continuing with a warning or something?

This is intended to flag a malformed action list - effectively, what I'm trying to communicate is that if the ChangeStepSize action is used, the UpdateU action must also be used to permit the action-list control flow (instead of stepping as a subroutine e.g. in ComputeTimeDerivatives)
But, now that you point it out, I see that this would be better handled with a static assert, so I'll squash that in too.

When a step is rejected and using `take_step`, the step is retried with new step values until the step is accepted.

moxcodes · 2021-03-18T19:22:15Z

Squashed in converting the action check to a static_assert, and minor changes to the corresponding test to pass the static assert.

nilsdeppe reviewed Feb 10, 2021

View reviewed changes

wthrowe requested changes Feb 10, 2021

View reviewed changes

moxcodes mentioned this pull request Feb 19, 2021

Reorganize time stepper history management #2883

Closed

3 tasks

wthrowe mentioned this pull request Mar 1, 2021

Remove self-start step-action introspection #2935

Merged

3 tasks

moxcodes force-pushed the error_stepper_action_reorder branch 2 times, most recently from 7e617ae to f32a6d7 Compare March 9, 2021 18:55

wthrowe requested changes Mar 10, 2021

View reviewed changes

wthrowe reviewed Mar 12, 2021

View reviewed changes

moxcodes force-pushed the error_stepper_action_reorder branch from 1e436a1 to 48a3f5d Compare March 12, 2021 06:22

moxcodes added the in progress Don't review, used for sharing code and getting feedback label Mar 12, 2021

moxcodes force-pushed the error_stepper_action_reorder branch from 48a3f5d to 41e4795 Compare March 12, 2021 18:31

wthrowe reviewed Mar 12, 2021

View reviewed changes

moxcodes force-pushed the error_stepper_action_reorder branch from 7c237ec to 174e489 Compare March 12, 2021 22:40

wthrowe requested changes Mar 12, 2021

View reviewed changes

moxcodes force-pushed the error_stepper_action_reorder branch from 174e489 to 569f5f9 Compare March 12, 2021 22:46

wthrowe previously approved these changes Mar 12, 2021

View reviewed changes

nilsdeppe requested changes Mar 15, 2021

View reviewed changes

moxcodes dismissed wthrowe’s stale review via 4ccd26a March 16, 2021 00:04

moxcodes force-pushed the error_stepper_action_reorder branch 2 times, most recently from 4ccd26a to 2966cb8 Compare March 16, 2021 00:24

moxcodes force-pushed the error_stepper_action_reorder branch from 2966cb8 to 709cc00 Compare March 16, 2021 00:27

nilsdeppe reviewed Mar 16, 2021

View reviewed changes

moxcodes added 7 commits March 15, 2021 20:51

Factor time-stepper actions into free functions

96ab3ae

This permits the stepper procedure to be called as a subroutine of other actions or functions

Move is_a_lambda to TypeTraits

4fea211

Add step rejection to CFL chooser

d82f8ac

Reorder step change to before AdvanceTime

b740964

Move initialize_history to helper interface

4a3a9ca

It's useful for testing other time utilities that need a prepared history for AdamsBashforthN

Add take_step function

d19c663

For ease of use in actions which need to invoke the stepper as a subroutine.

moxcodes force-pushed the error_stepper_action_reorder branch 2 times, most recently from 1beb71d to f9f6182 Compare March 16, 2021 04:09

nilsdeppe previously approved these changes Mar 16, 2021

View reviewed changes

wthrowe requested changes Mar 16, 2021

View reviewed changes

wthrowe removed the in progress Don't review, used for sharing code and getting feedback label Mar 16, 2021

moxcodes dismissed nilsdeppe’s stale review via b618efe March 18, 2021 06:11

moxcodes force-pushed the error_stepper_action_reorder branch from f9f6182 to b618efe Compare March 18, 2021 06:11

nilsdeppe previously approved these changes Mar 18, 2021

View reviewed changes

wthrowe reviewed Mar 18, 2021

View reviewed changes

Allow step rejection for LTS schemes

7d5c23b

When a step is rejected and using `take_step`, the step is retried with new step values until the step is accepted.

moxcodes dismissed nilsdeppe’s stale review via 7d5c23b March 18, 2021 19:21

moxcodes force-pushed the error_stepper_action_reorder branch from b618efe to 7d5c23b Compare March 18, 2021 19:21

wthrowe approved these changes Mar 18, 2021

View reviewed changes

nilsdeppe approved these changes Mar 18, 2021

View reviewed changes

kidder merged commit dc4cde4 into sxs-collaboration:develop Mar 20, 2021

nikwit mentioned this pull request Mar 24, 2021

Add executable for CurvedScalarWave #2973

Closed

3 tasks

Error stepper action reorder #2862

Error stepper action reorder #2862

Uh oh!

Conversation

moxcodes commented Feb 10, 2021

Proposed changes

Upgrade instructions

Code review checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wthrowe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

moxcodes Feb 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

moxcodes commented Feb 10, 2021

Uh oh!

moxcodes commented Mar 9, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

moxcodes commented Mar 11, 2021

Uh oh!

wthrowe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

moxcodes commented Mar 12, 2021

Uh oh!

moxcodes commented Mar 12, 2021

Uh oh!

moxcodes commented Mar 12, 2021

Uh oh!

wthrowe left a comment

moxcodes Feb 10, 2021 •

edited

Loading