Parallel-in-Time / pySDC

pySDC is a Python implementation of the spectral deferred correction (SDC) approach and its flavors, esp. the multilevel extension MLSDC and PFASST.
http://www.parallel-in-time.org/pySDC
BSD 2-Clause "Simplified" License
32 stars 35 forks source link

Spread initialization for non-autonomous problem #393

Closed tlunet closed 4 months ago

tlunet commented 7 months ago

About the spread initialization in the base Sweeper class. Why should the evaluation of the RHS depend on the node time ? cf :

https://github.com/Parallel-in-Time/pySDC/blob/2f72ea3b923bfd58b2ede884f003416885a32fd7/pySDC/core/Sweeper.py#L348-L351

Not so sure how this was already tested for non-autonomous problem, but it makes a huge difference when computing with the Prothero-Robinson test case (stiff problem). For instance, this is an illustration error VS dt for some diagonal preconditioner, supposed to be good for stiff problems :

grafik

And now, considering just a copy of the initial solution AND RHS evaluation at each nodes, meaning replacing the last line in the previous code by :

L.f[m] = P.eval_f(L.u[m], L.time)

we get the following error VS dt graphs :

grafik

So, kind of a huge difference ... is there a reason why the RHS values should not simply be copied for all the nodes then ?

pancetta commented 7 months ago

Hm, interesting. So, I guess it does not make sense to change the time but not the input variable. Technically, this part also allows to write your own predictor (e.g. an IMEX Euler step) and then being able to change both would make sense.

tlunet commented 7 months ago

How should I proceed ? Should I add and other type of initialization like this

# copy u[0] to all collocation nodes, evaluate RHS
if self.params.initial_guess == 'spread':
    L.u[m] = P.dtype_u(L.u[0])
    L.f[m] = P.eval_f(L.u[m], L.time + L.dt * self.coll.nodes[m - 1])
# copy u[0] and RHS evaluation to all collocation nodes
elif self.params.initial_guess == 'copy':
    L.u[m] = P.dtype_u(L.u[0])
    L.f[m] = P.dtype_f(L.f[0])

hence differentiating spread and copy initialization in pySDC ? Or simply remove the time dependency in spread ?

brownbaerchen commented 7 months ago

Whatever you decide, please make the same changes to the MPI sweeper here.

pancetta commented 7 months ago

How should I proceed ? Should I add and other type of initialization like this

# copy u[0] to all collocation nodes, evaluate RHS
if self.params.initial_guess == 'spread':
    L.u[m] = P.dtype_u(L.u[0])
    L.f[m] = P.eval_f(L.u[m], L.time + L.dt * self.coll.nodes[m - 1])
# copy u[0] and RHS evaluation to all collocation nodes
elif self.params.initial_guess == 'copy':
    L.u[m] = P.dtype_u(L.u[0])
    L.f[m] = P.dtype_f(L.f[0])

hence differentiating spread and copy initialization in pySDC ? Or simply remove the time dependency in spread ?

I think I'd prefer this. But then put the default to copy.

tlunet commented 7 months ago

Just hope it won't brake any test for which the results are based on the default using spread :sweat_smile: (like for Allen-Cahn, for instance ...)

pancetta commented 7 months ago

We shall see!

tlunet commented 7 months ago

Changing default prediction from spread to copy does break many tests : as soon as there is some forcing in the problem RHS, then this changes the results.

For instance, in pySDC/tests/test_benchmarks/test_PFASST_NumPy.py, the spread prediction allows to converge a bit faster (make sense, since it is using the forced heat equation problem, and spread is more accurate in this case ...), so changing to copy makes PFASST converge in a mean of 5.5 iterations instead of 5.

So what should I do @pancetta ?

  1. let spread as default in the sweeper base class
  2. correct all the tests that depend on the prediction so they force the spread initialization
pancetta commented 7 months ago

I guess the simplest solution would be to keep spread as the default and deviate from this when needed? Least surprises, maybe?

brownbaerchen commented 4 months ago

Can we close this issue? @tlunet

tlunet commented 4 months ago

Yes, that was solved with #397