Closed ddundo closed 4 months ago
Sorry, I only just saw this issue. In future, please tag me if you want a response. I haven't really thought about it to be honest - probably because there wasn't an issue for it (until now).
For the forward solution, the solution dictionary doesn't have the initial condition in it. But it does have some subset of the solutions at timesteps, depending on the export frequency. In the proposed ODE demo, the export frequency is 1 so all the timesteps get put in there. Presumably the case you are considering has export frequency > 1?
Hmm thanks @jwallwork23, this is quite different then from how I thought it worked! And I really remember us talking about needing to come up with a nice way to save the final solution in the solution dictionary (you said you might just make the final subinterval solution list longer - if this rings a bell). Have you made some changes to this or has it stayed the same?
So if I understood you right, let's say we have a total of 20 timesteps, 2 subintervals, and an export frequency of 5. So the solution dictionary will have forward solutions at timesteps t [[t5, t10], [t15, t20]]? This is how it seems to be in the ODE example too. I thought it was [[t0, t5], [t10, t15]], i.e. the final solution at t20 missing - that's why the ODE example surprised me.
Okay maybe the simplest fix is to do this is to just remove the export frequency altogether, meaning all solution fields get exported. Then we leave it to the user to decide what they keep/plot. The export frequency was introduced to account for the case where solution fields are memory intensive, but I don't think anyone's actually hitting that issue yet so it was probably premature.
What do you think @ddundo? If happy with this solution, I can take this on.
I dug into this to understand how it works - I think I get it now. If we have 8 timesteps in total and 2 subintervals, with num_timesteps_per_export=1
, the solution dictionary will have solutions at timesteps t [[t1, t2, t3, t4], [t5, t6, t7, t8]]
, where t8 is the final solution. (note t1 is not the initial condition)
With num_timesteps_per_export=2
, the sol dict is [[t1, t3], [t5, t7]]
. I think it would make much more sense if in this case it would be [[t2, t4], [t6, t8]]
, so that we have the final solution at each subinterval.
I would prefer to keep the export frequency option and just modify it to have the above behaviour. What do you think @jwallwork23?
Potentially even add a separate list to hold initial conditions for each subinterval. I.e. the list would store [t0, T4]
, where t0
is the initial condition and where T4
is the projected t4
solution on the mesh of the second subinterval.
That sounds like a good solution to me!
We discussed adding the final solution (and indicator) in the solution dictionary several times but I'm not sure if the agreement was reached in the end - I couldn't find an open issue about this. But now I took a look at the ODE PR and it looks like the solutions dictionary there has all the exported solutions, including the final one, but not the initial one. And it's not obvious to me how this was done.
So I was wondering what's the official status on this? :)