mcsiple/mmrefpoints

Functionality changes from HY JOSS review

Closed this issue · 2 comments

mcsiple commented

From Hao Ye:

  • I think arrange needs to be added to the functions to importFrom dplyr on line 15 of 17_plot_proj.R and line 20 of 18_multiplot_proj.R
  • For the model projection plot, it might be useful to use one of the interactive plotting packages like plotly, which would let the user zoom in on certain portions (to better see distinction between trajectories or better identify when extirpation is predicted). In addition, the legend for Bycatch level only has the labels for "{High end / Midpoint / Low end} of bycatch range", and it may be useful to calculate the numbers from the specified bycatch mortality range to make the legend more informative by itself.
  • When toggling between bycatch mortality by "Numbers per year" and "Mortality rate", the interface for selecting Bycatch CV changes between a text box and a slider. Is there a reason for this?
  • The kite plot in the "Advanced Projections" seems to occasionally error out with a "need finite 'ylim' values" message. (e.g. selecting "Sea lion", leaving all parameters at default values, and clicking on "Run projections")
  • The population time series plot seems to fail when both "Show individual projections" and "Show multiple depletion levels" is checked.
  • It'd be ideal to reorder the bycatch level factor so that Low, Midpoint and High are in either increasing or decreasing order.
  • The violin plots are flat for some bycatch levels, resulting in horizontal lines instead of a distribution. There might be a way to indicate such, and perhaps to better show grouping for "bycatch level" within "years after projections start"
  • The design is a bit awkward with the side-by-side of parameters and projection plots. I feel as though I would prefer to have the usage steps proceed from top to bottom, so that I set parameters in a section, then scroll down and click to run projections, then scroll down and see model outputs.
  • You have different kinds of summary outputs for the model projections. It might be nice to add an option to download the raw projection data for someone to do their own calculations.
  • The GAMMS link in the "PBR & PBR calculator" tab doesn't seem to work.
  • For the "Solve for Bycatch" tab, it would be ideal to show all the life history and other model parameters from the Advanced tab here, just so someone using it can confirm that the numbers are correct without having to switch tabs. I also think showing the table of tested bycatch rate values would be good, as it would allow people to then go back to the Advanced tab and test/confirm the results.
  • It does seem a bit confusing that the input arguments to dynamics() have separate arguments for the life history parameters, whereas in projections(), they're given as a named list. I think this could be made more consistent, and that the list structure is probably better as it is easier to refer to as a single object to pass to multiple functions.
mcsiple commented

Fixed multiplot issue with commit baf8a3d

mcsiple commented

I have addressed all but two of the above:

  1. The bycatch factor levels in the kite plot. The ggradar() function makes this weirdly difficult and I haven't figured out how to fix it. Since the legend colors are correctly assigned to bycatch levels I am leaving this for now, unless Hao feels strongly about it.
  2. The side by side design is a little weird before the projections are run, but it does allow the user to see the parameters they entered and the projections at the same time. I am open to others' opinion on this, but I am inclined to keep the sidebar-style layout. Maybe @tessabfrancis can help with some wisdom/opinion.

I changed dynamics() life history parameter inputs to a list with commit bf87157

Everything else is incorporated now and commits have been tagged both here and in Hao Ye's original review issue, #54 .