Add time stepper that allows OpenFOAM to control the time step so that CFL adaptive stepping can be used by mattfalcone1997 · Pull Request #48 · aurora-multiphysics/hippo

mattfalcone1997 · 2025-05-01T15:14:12Z

Summary

Implements FoamControlledTimeStepper which allows OpenFOAM to control the time step with MOOSE inserting time steps for sub-cycling synchronisation. Currently, synchronizeAdaptiveTimes is added to FoamSolver and called during run to ensure the MOOSE time and OF time is synchronised. However, it is not clear if this is the best design choice.

Related Issue

This should at least partially resolves #28

Checklist

Initial implementation
Basic test to show that the correct time steps are used if OpenFOAM controls the timestep whether It's adjustable or not.
Consider if there is a better approach to synchronise times (Can the OF maxDeltaT function object be modified).
Add additional warnings and errors if parameters such as dt are passed to the executioner.
Can an explicit test be added to verify the adaptive time step computed by MOOSE is the same as OpenFOAM

…pping

hsaunders1904

Looks good and getting OpenFOAM to do an extra synchronisation step seems reasonable to me. I haven't had a chance to do any manual testing yet, but I've left a few comments.

hsaunders1904 · 2025-05-02T08:39:52Z

include/solvers/timesteppers/FoamControlledTimeStepper.h

+
+/*
+Time stepper that allows OpenFOAM to control the time step enabling features such as CFL
+ daptive time steps. the intention is to allows the current time step in OpenFOAM


Suggested change

daptive time steps. the intention is to allows the current time step in OpenFOAM

adaptive time steps. The intention is to allows the current time step in OpenFOAM

hsaunders1904 · 2025-05-02T08:47:55Z

src/timesteppers/FoamControlledTimeStepper.C

+  if (deltaT >= Foam::rootVGreat)
+    mooseError("Computed OpenFOAM time step must be less that rootVGreat");


In what scenario would deltaT be greater than rootVGreat? Could we consider clipping the value to rootVGreat? A comment here explaining what's gone on and why an error is the best way to deal with this case, would be helpful.

Might also be nice for the user if the values of deltaT and rootVGreat were included in this error message.

I should probs add some more comments, but this is taken from OpenFOAM (need to remember where). But in some cases (like the buoyancy case), the deltaT becomes huge with adaptive time stepping on the first time step. I think rootVGreat is like 1e158, so I think if it suggests that we probs want the user to reconsider. I can add those numbers to the error.

hsaunders1904 · 2025-05-02T09:05:15Z

test/tests/timesteppers/foam_controlled_tstep_insert/verify.sh

+for t in 0.4 0.8 1 1.4 1.8 2 2.4 2.8 3; do
+    if ! [ -d "${CASE_DIR}/$t" ]; then
+        echo "Error for t=$t"
+        exit 1
+    fi
+done


This checks we have the directories we want. Should we also check that time directories we don't want, do not exist?

Maybe we can count the number of time directories and make sure that's correct as well? Something like find buoyantCavity -maxdepth 1 -type d | grep -E '/[0-9]+(\.[0-9]+)?$' | wc -l will count the number of time directories.

Makes sense

hsaunders1904 · 2025-05-02T09:15:40Z

include/solvers/timesteppers/FoamControlledTimeStepper.h

+#include "InputParameters.h"
+


This is a MOOSE include so should go in the include block below and use <> brackets.

I should have documented the include conventions for the repo (my bad). But Hippo includes are first (using ""). Then third party includes (so MOOSE/OpenFOAM, using <>), then standard library includes (using <>). Not massively important, but it keeps things organised and easy to parse by eye.

Suggested change

#include "InputParameters.h"

#include <InputParameters.h>

hsaunders1904 · 2025-05-02T09:18:31Z

src/timesteppers/FoamControlledTimeStepper.C

+#include "scalar.H"
+


Suggested change

#include "scalar.H"

#include <scalar.H>

mattfalcone1997 · 2025-05-02T10:56:30Z

@hsaunders1904 An additional concern atm is that an addition time is inserted by MOOSE on the OpenFOAM solve based on

FoamSolver::synchronizeAdaptiveTimes(double dt)
{
  double foam_dt = runTime().deltaTValue();
  if (std::abs(dt - foam_dt) > 1e-6)
  {
    runTime().setDeltaTNoAdjust(dt);
  }
}

Here 1e-6 is fixed, which isn't ideal. However, I can't use MOOSE timestepTol as FoamSolver cannot access it. Have you any ideas how to improve this?

hsaunders1904 · 2025-05-06T12:42:03Z

@hsaunders1904 An additional concern atm is that an addition time is inserted by MOOSE on the OpenFOAM solve based on
FoamSolver::synchronizeAdaptiveTimes(double dt)
{
  double foam_dt = runTime().deltaTValue();
  if (std::abs(dt - foam_dt) > 1e-6)
  {
    runTime().setDeltaTNoAdjust(dt);
  }
}
Here 1e-6 is fixed, which isn't ideal. However, I can't use MOOSE timestepTol as FoamSolver cannot access it. Have you any ideas how to improve this?

Where are you calling synchronizeAdaptiveTimes? Can you add a tolerance argument and pass through timestepTol?

mattfalcone1997 · 2025-05-06T12:47:51Z

@hsaunders1904 It's called in the FoamSolver::run function. Currently I changed run to pass the MOOSE time step so I could also pass the time step tolerance. Just felt unnatural to pass such a 'low-level' argument to such a 'high-level' function, if this makes sense. That may be the only way to do it though without substantial changes to FoamSolver. If necessary, this can always be done later as a refactor.

hsaunders1904 · 2025-05-06T13:07:33Z

Remind me where timeStepTol comes from? Could you add _timestep_sync_tol (or similar, with a default) as a member to FoamSolver, with a corresponding setTimestepSyncTol() function that you can call from the FoamProblem? FoamProblem is largely the bridge between MOOSE stuff and the MOOSE-less FoamSolver class.

mattfalcone1997 · 2025-05-06T13:10:35Z

Remind me where timeStepTol comes from? Could you add _timestep_sync_tol (or similar, with a default) as a member to FoamSolver, with a corresponding setTimestepSyncTol() function that you can call from the FoamProblem? FoamProblem is largely the bridge between MOOSE stuff and the MOOSE-less FoamSolver class.

That should be doable

hsaunders1904 · 2025-05-06T13:13:00Z

FoamProblem might even be a better place for the synchronizeAdaptiveTimes logic in the first place. It's FoamProblem's job to do synchronization.

mattfalcone1997 · 2025-05-06T13:16:36Z

FoamProblem might even be a better place for the synchronizeAdaptiveTimes logic in the first place. It's FoamProblem's job to do synchronization.

The is issue is that it must be called after adjustDeltaT in run, otherwise OpenFOAM will do its own thing. The other potential way to do it is to modify OpenFOAM's maxDeltaT function object if possible to force synchronisation if necessary.

…bject

mattfalcone1997 · 2025-05-07T15:58:13Z

@hsaunders1904 I have just shown another way of doing this using OpenFOAM's function objects. It means that information about the timestep doesn't need to be passed through Foam::Solver, but is generally a bit more obscure. Also, I have found that to use an up-to-date time step preSolve needs to be called in computeDT. Probably not too expensive tho

hsaunders1904

From what I can work out, this looks good. I haven't quite got my head around how the functionObject is solving the problem we had, yet. But I probably just need to spend a bit more time looking at it.

hsaunders1904 · 2025-05-12T14:57:01Z

include/solvers/timesteppers/FoamControlledTimeStepper.h

+
+/*
+Time stepper that allows OpenFOAM to control the time step enabling features such as CFL
+ daptive time steps. The intention is to allows the current time step in OpenFOAM


Suggested change

daptive time steps. The intention is to allows the current time step in OpenFOAM

adaptive time steps. The intention is to allow the current time step in OpenFOAM

This comment/docstring should probably be just above FoamControlledTimeStepper's definition

hsaunders1904 · 2025-05-12T16:11:11Z

src/timesteppers/FoamControlledTimeStepper.C

+  // Not ideal, but for MOOSE to get an accurate deltaT
+  // preSolve must be called as this updates the BCs.
+  Foam::solver & foam_solver{solver().solver()};
+  Foam::pimpleSingleRegionControl pimple(foam_solver.pimple);
+
+  pimple.read();
+  foam_solver.preSolve();


I reckon this should probably be moved into a function in FoamSolver, then it's just solver()->preSolve() and we keep all the OpenFOAM stuff wrapped up behind an interface. I feel like that interface could become very useful when we start looking at getting Hippo working with OpenFOAM.com.

Also, doesn't the solver already have an associated pimple object (foam_solver.pimple)? Does foam_solver.pimple.readIfModified() work?

I will do this, I think I will also create a FoamSolver::computeDeltaT function to hide the actual calculation of the OpenFOAM time step behind the interface

hsaunders1904 · 2025-05-12T16:16:15Z

src/timesteppers/FoamControlledTimeStepper.C

+  _dt_adjustable = foam_solver.runTime.controlDict().lookupOrDefault("adjustTimeStep", false);
+
+  // Add functionObject that tells OpenFOAM what MOOSE's time step is.
+  // If MOOSE inserts a timestep the functionObjects.maxTime() with return


Suggested change

// If MOOSE inserts a timestep the functionObjects.maxTime() with return

// If MOOSE inserts a timestep then functionObjects.maxTime() will return

maxTime() or maxDeltaT()?

mattfalcone1997 · 2025-05-13T08:22:32Z

From what I can work out, this looks good. I haven't quite got my head around how the functionObject is solving the problem we had, yet. But I probably just need to spend a bit more time looking at it.

It is a little obscure, but the key idea is that runTime.functionObjects().maxDeltaT() in adjustDeltaT basically loops over the function objects and chooses the minimum, so by having a function Object that returns what MOOSE wants, OpenFOAM will use the MOOSE time step if it is smaller than what OpenFOAM wants. As a result, if MOOSE wants to add a synchronisation step OpenFOAM will also use it too.

hsaunders1904 · 2025-05-13T09:23:51Z

From what I can work out, this looks good. I haven't quite got my head around how the functionObject is solving the problem we had, yet. But I probably just need to spend a bit more time looking at it.

It is a little obscure, but the key idea is that runTime.functionObjects().maxDeltaT() in adjustDeltaT basically loops over the function objects and chooses the minimum, so by having a function Object that returns what MOOSE wants, OpenFOAM will use the MOOSE time step if it is smaller than what OpenFOAM wants. As a result, if MOOSE wants to add a synchronisation step OpenFOAM will also use it too.

Would be great to have this in a comment. Then I think we're good to go!

…amSolver

mattfalcone1997 · 2025-05-13T11:26:07Z

@hsaunders1904 The comment has been added. I have also further attempted to improve the hiding of the gory OpenFOAM bits within FoamSolver.

hsaunders1904

Great to see this working! 🚀

This isn't for this PR, but would the old FoamTimestepper also benefit from using your new functionObject? The problem that #42 aimed to solve is still around

mattfalcone1997 added 4 commits April 29, 2025 17:10

Implement basic version of OpenFOAM controlled time stepping

9bc4ad4

Fix core functionality of initial implementation of adaptive time ste…

e0679a9

…pping

Fix bug to ensure initial delta t is used if OF not adaptive

6e26468

Add basic tests for FoamControlledTimeStepper

7cce0a1

mattfalcone1997 requested a review from hsaunders1904 May 1, 2025 15:14

mattfalcone1997 added 2 commits May 1, 2025 16:37

Fix formatting issues

d28d8e5

Fix bug in CFL time stepper test

34bdfc8

mattfalcone1997 marked this pull request as ready for review May 2, 2025 09:06

hsaunders1904 reviewed May 2, 2025

View reviewed changes

mattfalcone1997 added 2 commits May 2, 2025 10:34

Remove unnecesary OpenFOAM directory from test

4d3a85d

Fix formating and tests in response to review

ac90f45

Add more informative comments to the time stpper

4e48e12

mattfalcone1997 requested a review from hsaunders1904 May 2, 2025 16:18

Showcase a different approach where deltaT is imposed using functionO…

45cb46a

…bject

hsaunders1904 reviewed May 12, 2025

View reviewed changes

Improve encapsulation of Foam functions and comment fixes

fa162d3

mattfalcone1997 requested a review from hsaunders1904 May 13, 2025 08:51

Further improve encapsulation of time stepper by hiding details in Fo…

f6e798f

…amSolver

hsaunders1904 approved these changes May 13, 2025

View reviewed changes

mattfalcone1997 merged commit f1481d5 into aurora-multiphysics:main May 19, 2025
2 checks passed

mattfalcone1997 deleted the cfl_time_stepper branch June 13, 2025 15:13

	daptive time steps. the intention is to allows the current time step in OpenFOAM
	adaptive time steps. The intention is to allows the current time step in OpenFOAM

		if (deltaT >= Foam::rootVGreat)
		mooseError("Computed OpenFOAM time step must be less that rootVGreat");

	// If MOOSE inserts a timestep the functionObjects.maxTime() with return
	// If MOOSE inserts a timestep then functionObjects.maxTime() will return

Conversation

mattfalcone1997 commented May 1, 2025

Summary

Related Issue

Checklist

Uh oh!

hsaunders1904 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!

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!

mattfalcone1997 commented May 2, 2025

Uh oh!

hsaunders1904 commented May 6, 2025

Uh oh!

mattfalcone1997 commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hsaunders1904 commented May 6, 2025

Uh oh!

mattfalcone1997 commented May 6, 2025

Uh oh!

hsaunders1904 commented May 6, 2025

Uh oh!

mattfalcone1997 commented May 6, 2025

Uh oh!

mattfalcone1997 commented May 7, 2025

Uh oh!

hsaunders1904 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!

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!

mattfalcone1997 commented May 13, 2025

Uh oh!

hsaunders1904 commented May 13, 2025

Uh oh!

mattfalcone1997 commented May 13, 2025

Uh oh!

hsaunders1904 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mattfalcone1997 commented May 6, 2025 •

edited

Loading