search

SABS-R3-Epidemiology / seirmo

This is a project to model the outbreak of an infectious disease with the SEIR model.
BSD 3-Clause "New" or "Revised" License
2 stars 2 forks source link

Add plotters #107

Closed Elizabeth-Hayman closed 2 years ago

Elizabeth-Hayman commented 2 years ago

Added a configurable plotter to the plot_from_numpy. Changed name of existing plotting so that's now in a separate file. Big edits on the example_stoch_flow, want to keep most of that.

codecov[bot] commented 2 years ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (8f02628) to head (1401951).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## AddProspensity #107 +/- ## =================================================== + Coverage 97.18% 100.00% +2.81% =================================================== Files 14 14 Lines 462 513 +51 =================================================== + Hits 449 513 +64 + Misses 13 0 -13 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

KCGallagher commented 2 years ago

The last commit is a significant functionality change! Currently, if the user does not explicitly show a figure generated by the ConfigurablePlotter class, it remains open indefinitely (I think?). I don't know how best to implement this to avoid RunTime warnings and performance loss, but currently I have added functionality to begin() that closes all open figures initially, to prevent multiple figures being open simultaneously.

I'm not convinced this is the best approach - for example maybe we could close figures after saving instead, but I can think of cases where either approach might result in unwanted functionality. Either way, best thought about and handled explicitly I think?

EDIT: Added a delete magic method instead which closes current plot when the instance of the class is removed - this is much tidier (thanks NF for the idea)

KCGallagher commented 2 years ago

I have written cursory tests for _stochastic_sim_plotter, however I am not sure if it adds much now we have the wonderful new ConfigurablePlotter class (and I am concerned it doesn't actually match the new data collector?) - might you be able to advise @Elizabeth-Hayman ?

Edit: This class has now been removed from the model and documentation