Skip to content

Feat multi model processing - #477

Open
JZahner1 wants to merge 21 commits into
nmfs-ost:mainfrom
JZahner1:feat-multi-model-processing
Open

JZahner1 wants to merge 21 commits into
nmfs-ost:mainfrom
JZahner1:feat-multi-model-processing

Conversation

@JZahner1

@JZahner1 JZahner1 commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

What is the feature?

  • Enables convert_output to handle multiple models in sequence. This changes from the previous way multiple models were handled (passing a list of tibble outputs from convert_output, one per model to each plotting function), but preserves the previous funtionality that allows for plotting multiple models simultaneously. Models are appended together based on a model column now present in all convert_output tibble.

This change should improve memory usage, per #457.

How have you implemented the solution?

  • This was implemented by moving the existing convert_output workflow into a purrr::imap function call, allowing for the function to me run sequentially over a list of model files inputs as opposed to a single file.

Does the PR impact any other area of the project, maybe another repo?

PR contribution checklist

Thank you for contributing to stockplotr! Please check the boxes below to confirm this pull request (PR) is ready for review. Edit this PR description to add the release indicator and any explanations.

  • I have read and adhered to the contributor guide.
  • I have kept this PR focused on one plot where applicable. For multiple features, I have used separate PRs (stacked PRs are welcome if needed).
  • I have aimed to keep this PR to no more than 750 lines, excluding tests and example data (for example, up to 750 lines for a new function).
  • If I have provided an example dataset, I have explained why the existing sample datasets are not sufficient.
  • I have tested that my changes are compatible with all example outputs in the "data" folder (example_data.rda; pollock.rda; ss3_simple.rda; vermilion_snapper.rda).
  • If I didn't follow the development workflow, I have explained why in this PR.
  • If applicable, I've added the plot to save_all_plots() and updated the associated tests in test-save_all_plots.R.
  • After opening this PR, I have checked that my own tests and spellcheck pass (spelling::spell_check_package()). If other checks fail, I have flagged them for the package maintainers to help diagnose and remedy.
  • I have requested review from one package maintainer (Sam or Sophie).

pass a list of model files and get back a single tibble object of all models concatenated together
Test against two SS3 models and a combination of SS3 and Rceattle models.
Multimodel data processing is now handled natively by `convert_output`
Verify that `filter_data` works on two models and that ` plot_spawning_biomass` generates a plot with two lines.
@JZahner1

JZahner1 commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

I believe this still requires a decision regarding how to handle key quantity exporting and caption/alt text when multiple models are present in a plot.

@JZahner1
JZahner1 requested review from Schiano-NOAA and sbreitbart-NOAA and removed request for Schiano-NOAA November 12, 2026 20:30
@JZahner1

Copy link
Copy Markdown
Collaborator Author

@sbreitbart-NOAA @Schiano-NOAA I think this is ready to review, minus the issues about multi-model key quantities and captions/alt-text but I think that needs to be a separate PR anyway.

@JZahner1

Copy link
Copy Markdown
Collaborator Author

As a note, when plotting multiple models, all models need to have the same module plotted at a given time. This is a problem now where each model type (SS3, Rceattle, BAM, etc.) each have slightly different modules associated with particular labels, but this should be standardized across models at some point.

@Schiano-NOAA

Copy link
Copy Markdown
Collaborator

Sounds good! This is getting put in my queue along with your other PR. Hoping to get to it Monday

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants