PredictiveEcology / Biomass_speciesParameters

Other
1 stars 4 forks source link

plotting generates identical figures with different filenames #32

Open CeresBarros opened 1 year ago

CeresBarros commented 1 year ago

Plotting in Biomass_speciesParameters@dev-stable is currently occurring during the init event via:

    Plots(gg, usePlot = FALSE, fn = print, ggsaveArgs = list(width = 10, height = 7),
          filename = paste("Pairwise species fits ", gsub(":", "_", sim$._startClockTime)))

meaning that each time the init is run a new figure file is created, because sim$._startClockTime will differ.

I can see the interest of doing so during module development phase, but when using this module in combination with others and when event caching is off, this creates a lot of duplicated figure files.

Is there a reason why we shouldn't overwrite the same figure with an update version?

achubaty commented 1 year ago

I agree that it not generally desirable, so I'm happy to have the clock time removed from the file name.

CeresBarros commented 1 year ago

I'd also add that "Pairwise species fits" is not entirely what the plots are showing (a comparison between LandR growth curves and growth curves from fitted non-linear models) -- and spaces in file names aren't great either. Could we also change the figure file names?

achubaty commented 1 year ago

Yes, file names should be descriptive. Can you also ensure file names are prefixed by the module name? It makes it easier to associate specific figures with certain modules (and to find them as a human).

CeresBarros commented 1 year ago

Can you also ensure file names are prefixed by the module name?

I partially agree with this. One the one hand, yes, appending the module name would facilitate finding figures and even interpreting them. However,

  1. it feels strange to me to do this for one module only
  2. If we propagate across modules that will mean changing many figure names
  3. in some cases, we may end up with very long filenames.

I wonder if, instead, we could (in this and and other modules in which to do this) create a module parameter that governs where figures are saved (e.g. P(sim)$.plotsPath). By default, Plots saves to a file.path(outputPath(sim), "figures") folder. This could still be the default P(sim)$.plotsPath, but the user could change that to file.path(outputPath(sim), "figures", "<moduleName>").

This partly addresses my point no. 1, in that the figure names would remain the same. It doesn't address no 2, as it still means changing internal code of several modules (and creating a new parameter), if we want to propagate this across modules. It fully addresses no 3, as filenames would remain as short/long as they currently are

achubaty commented 1 year ago

I think separate subdirs for each module makes more sense. That could be an easy addition to SpaDES.core - provide an accessor to the module's figure dir so it doesn't need to specified manually, and it should be easy enough to propagate this change through each module.

CeresBarros commented 1 year ago

In that case I'll hold off creating any parameters for Biomass_speciesParameters or changing the filename above, other than removing clocktime

CeresBarros commented 1 year ago

Actually, scratch that. I'll still suggest/PR a more accurate filename wrt what the figure is about.

achubaty commented 1 year ago

I implemented figurePath() for simList objects (https://github.com/PredictiveEcology/SpaDES.core/pull/249), which becomes the default mechanism used by Plots() when writing files.

When called from a module, figurePath(sim) will return the value of file.path(outputPath(sim), "figures", moduleName). When not called from a module, it simply drops the moduleName component.