Improve visualization tools with a video generation

[albangossard]

• Add a function that generates a mp4 video of the ice thickness over time
• This requires to add a complete_report flag in the simulation parameters which breaks retro-compatibility

There is a PR in Huginn associated to this one: ODINN-SciML/Huginn.jl#64
Since this new feature is not that important for future developments, I suggest that even if we merge the current PR, we don't waste time making a new release. We'll be able to merge ODINN-SciML/Huginn.jl#64 later on once a new release of Sleipnir is fully justified.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 66.66667% with 12 lines in your changes missing coverage. Please review.

Project coverage is 69.95%. Comparing base (0b3536a) to head (3cac015).

Files with missing lines	Patch %	Lines
src/simulations/results/results_utils.jl	0.00%	11 Missing ⚠️
...imulations/results/results_plotting_video_utils.jl	95.83%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main      #79       +/-   ##
===========================================
+ Coverage   35.11%   69.95%   +34.83%     
===========================================
  Files          17       18        +1     
  Lines         672      669        -3     
===========================================
+ Hits          236      468      +232     
+ Misses        436      201      -235     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[JordiBolibar]

Very cool @albangossard! This type of stuff will be very useful in the future to present the results of the simulations. We should iterate on all the existing plotting functions to improve them. I know Makie.jl also have 3D animations, so we could also try something in 3D in the future.

[JordiBolibar]

I'm not sure this flag is really self explainable. This means that the full solution is stored? Or which time step have you chosen? The solve function of OrdinaryDiffEq.jl already enables to control this. Is this also linked to the creation of the results with the video?

[facusapienza21]

Is it complete_result currently being used for something?

[albangossard]

@facusapienza21 my initial goal was to use complete_report in ODINN-SciML/Huginn.jl#64 to dump all the results of the solver and store them in the Results instance. As pointed out by @JordiBolibar we already have params.solver.save_everystep which controls whether the solve function returns all the intermediate steps or not. This was to be able to generate the mp4 video of the ice thickness over time.
In 5707b8a I removed complete_report and save_everystep will be used instead in ODINN-SciML/Huginn.jl#64

[facusapienza21]

This looks pretty cool! One question: would it make more sense to have this tools in another of the packages and leaving Sleipnir just for very basic computations? It would be good if Sleipnir is very light and maybe add this to Huginn or any other library. What do you think?

[albangossard]

Very cool @albangossard! This type of stuff will be very useful in the future to present the results of the simulations. We should iterate on all the existing plotting functions to improve them. I know Makie.jl also have 3D animations, so we could also try something in 3D in the future.

@JordiBolibar I didn't have the courage to update them all at once but yes the goal in the long term is to improve all the plotting functions so that we can generate videos.

This looks pretty cool! One question: would it make more sense to have this tools in another of the packages and leaving Sleipnir just for very basic computations? It would be good if Sleipnir is very light and maybe add this to Huginn or any other library. What do you think?

@facusapienza21 I agree that keeping Sleipnir as minimalist as possible is important. I added the new plotting functions in Sleipnir because there were already a few of them. I have no opinion on which library would be the good place to put all these functions. On one hand we don't need access to the solver in order to implement the plotting functions, but on the other hand we cannot test them without running simulations. So maybe we should move these functions to Huginn?

[JordiBolibar]

Very cool @albangossard! This type of stuff will be very useful in the future to present the results of the simulations. We should iterate on all the existing plotting functions to improve them. I know Makie.jl also have 3D animations, so we could also try something in 3D in the future.

@JordiBolibar I didn't have the courage to update them all at once but yes the goal in the long term is to improve all the plotting functions so that we can generate videos.

This looks pretty cool! One question: would it make more sense to have this tools in another of the packages and leaving Sleipnir just for very basic computations? It would be good if Sleipnir is very light and maybe add this to Huginn or any other library. What do you think?

@facusapienza21 I agree that keeping Sleipnir as minimalist as possible is important. I added the new plotting functions in Sleipnir because there were already a few of them. I have no opinion on which library would be the good place to put all these functions. On one hand we don't need access to the solver in order to implement the plotting functions, but on the other hand we cannot test them without running simulations. So maybe we should move these functions to Huginn?

We discussed this with Alban this morning, but I really think we should keep all the plotting utils together in Sleipnir.jl. Even if it is slightly less straightforward to test, it is more solid in terms of structure and code development. Otherwise, this opens the door to spreading utils and functionalities across all the packages, making it extremely difficult to navigate and to know where to add or edit code. Sleipnir has the Results object and all its plotting utils, so I strongly suggest this to be there.

[albangossard]

Hey @facusapienza21 @JordiBolibar finally I moved the video generation test in Sleipnir. This requires to save jld2 files in the repo but that's something we were already doing for other tests. These files are generated from a forward run in Huginn.
I have also put back the tests of plot_glacier

[facusapienza21]

Ok!!! Makes sense to me! I just wanted to bring the topic to the table in case it made sense to have plotting in other place. However, as @albangossard mentioned we don't have solvers here, so we will probably have to add some special plots that required the solver in other libraries (maybe for stuff that is not included in Results). We will definitely have more plotting in ODINN for tracking training, so I think it is then natural that plotting is distributed between libraries.

Feel free to merge!

[JordiBolibar]

Ok!!! Makes sense to me! I just wanted to bring the topic to the table in case it made sense to have plotting in other place. However, as @albangossard mentioned we don't have solvers here, so we will probably have to add some special plots that required the solver in other libraries (maybe for stuff that is not included in Results). We will definitely have more plotting in ODINN for tracking training, so I think it is then natural that plotting is distributed between libraries.

Feel free to merge!

Just to make sure we're on the same page, the outcome of this discussion is that ALL plotting functions will stay in Sleipnir, right? Because that's not what I understood from your last message haha.

[facusapienza21]

@JordiBolibar I think there are plotting functions that are in ODINN and don't make sense outside the training, which is not currently include in Sleipnir.