Moves the VTK dump after a solve.

[gnikit]

Fixes #304

This means that the solution is now displayed on the mesh on which it occured and not on the interpolated mesh after a spatial adapt.

A simulation_completed(current_time, timestep+1) had to be used for the cases where the option of final_timestep has been given to prevent double output in the last timestep.

This still leaves the adapt_at_first_timestep since by the time the first data dump occurs the mesh has already adapted, hence the 0th and 1st timestep dumps look the same mesh-wise.

TODO:

• Reorder the adapt_at_first_timestep to be after a data dump. That means that we need to:
  1. Obtain the timestep
  2. Move the check for VTK output
  3. Allocate and insert auxiliary fields
  4. Initialise the diagnostic fields
• I also think I need to delete some comments I added for the last time step data dump.

[stephankramer]

This means that the solution is now displayed on the mesh on which it occured and not on the interpolated mesh after a spatial adapt.

I'm afraid this is a feature rather than a bug. I can absolutely see the arguments of doing it either way: it makes sense to want to see the solution on the mesh that it was solved on, it also makes sense to want to look at the solution immediately after an adapt: many applications have complex interpolation/projection procedures where the result isn't obvious and needs inspecting. Also the solution after interpolation is the information that moves forward into the next timestep, so if you dump every timestep you get to see the state immediately before a failed timestep. Bear in mind that in the applications of fluidity from this main branch typically don't adapt every timestep but typically say every 20 timesteps - and I get the impression it maybe isn't in your case?

Of course we could introduce an option that allows you to do it either way, but I'm not sure the default should change. In particular the default for when the first dump happens with adapt-at-first-timestep should be after the adapt: in a typical setup the input mesh is very coarse and seeing the fields which are all prescribed on that mesh is completely useless; typically the mesh is just coarse enough that adaptivity can still tell where to put more resolution (in a first iteration), and only when the mesh has fully developed is it possible to see the various features of the initial condition that form the starting point of the numerical simulation.

I'm also generally very wary about pulling apart anything to do with the ordering in which things are done in a timestep. This is a prime example of "brittle code" with fluidity having had too many and varied applications all implementing various whistles and bells, and diagnostics with complex interdependencies, that are also impossible to test in all combinations. So if you want to introduce an option to do something different that's absolutely fine, as long as it is obvious that it doesn't affect existing functionality.

[gnikit]

I'm afraid this is a feature rather than a bug.

I see, this change was purely because of my work in radiation and what I perceive as optimal/logical. So I understand if there are cases where seeing the solution on the mesh on which the solve was performed might not be ideal.

In particular the default for when the first dump happens with adapt-at-first-timestep should be after the adapt

I am actually very happy for you to say this because I won't have to mess with the adapt-at-first-timestep output. When I first started this issue, I tried to get it to work and for the life of me I couldn't, it was a true nightmare trying to initialise all the dependencies before the 0th output. So I am happy to drop that bit.

So if you want to introduce an option to do something different that's absolutely fine, as long as it is obvious that it doesn't affect existing functionality.

That is completely understandable. I think it it only makes sense for me to add this only if you and the other devs think that this is a feature worth having in Fluidity, and it will be useful to someone down the road. Fluidity already has a massive code base and I wouldn't want to add to that features that make future maintenance harder.

If you and/or other devs think that it makes little difference whether or not the solution is output before or after an adapt, especially since a data dump is normally performed every few timesteps, feel free to close this PR.

[tmbgreaves]

Flagging that there is a proposed renaming of master branch to main on 26th May, which if this PR is still open at that point will require retargetting to main branch.

[gnikit]

I am closing this PR since for Fluidity specifically this does not make much of a difference due to the reason's stated above.