Skip to content

[feat]: Adds index intervals, follows PR #465 - #468

Merged
Schiano-NOAA merged 10 commits into
nmfs-ost:mainfrom
ericward-noaa:more-fims-plots-index-intervals
Nov 13, 2026
Merged

Schiano-NOAA merged 10 commits into
nmfs-ost:mainfrom
ericward-noaa:more-fims-plots-index-intervals

Conversation

@ericward-noaa

@ericward-noaa ericward-noaa commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

What is the feature?

Adding interval argument to plot_index
#' plot_index(
#' dat = stockplotr::example_data,
#' interval = 0.95,
#' log = TRUE,
#' interactive = FALSE
#' )

Follows #465

How have you implemented the solution?

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).

convert_output() passed FIMS output through unchanged, so `estimate` and
`year` were empty and no stockplotr plot could use it. The new
conout_fims() reshapes `FIMS::get_estimates()` output (or a FIMSFit object,
or an .rds file with either):

- Each data point (index, landings, age and length compositions) is split
  into "*_observed" and "*_predicted" rows, with the observation error and
  negative log-likelihood on the observed row, as for SS
- `estimated` becomes `estimate`, and the new `start_year` argument converts
  FIMS year indices to years; `era` is "time", or "fore" after the last year
  of data.
- fims_var_names.csv maps current FIMS labels (e.g., expected_recruitment,
  numbers_at_age, log_devs) to the standard labels.

Also fixes:
- convert_output() ignored a user-supplied `model` when `file` is an object.
- plot_index() matched every label containing "index" (e.g., index_log_q in
  Rceattle output or index numbers at age in FIMS output); it now plots only
  index_observed and index_predicted.
- plot_index() failed for a single index fleet, because the fleet facet was
  added back after process_data() dropped the column.
plot_index() adds `interval`, which draws intervals around the observed
index from its observation error, and `log`, which plots the index on a log
scale. Intervals assume lognormal errors, as in Stock Synthesis, FIMS, BAM,
and Rceattle: uncertainty labeled "se", "sd", or "log_sd" is used as the
log-scale standard error, and a CV is converted with sqrt(log(1 + CV^2)).
Both default to the current behavior (no intervals, linear scale).
@ericward-noaa ericward-noaa changed the title Adds index intervals [feat]: Adds index intervals, follows https://github.com/nmfs-ost/stockplotr/pull/465 Nov 6, 2026
@ericward-noaa ericward-noaa changed the title [feat]: Adds index intervals, follows https://github.com/nmfs-ost/stockplotr/pull/465 [feat]: Adds index intervals, follows PR #468 Nov 6, 2026
@ericward-noaa ericward-noaa changed the title [feat]: Adds index intervals, follows PR #468 [feat]: Adds index intervals, follows PR #465 Nov 6, 2026
Comment thread R/plot_index.R Resolved
Make default interval 0.95 per @iantaylor-NOAA
@ericward-noaa

Copy link
Copy Markdown
Contributor Author

@sbreitbart-NOAA @Schiano-NOAA This is ready for review

Comment thread R/plot_index.R Resolved
Comment thread R/plot_index.R Resolved

@Schiano-NOAA Schiano-NOAA left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Just one question to address and I will merge.

Fixed and better documented 3 logic pathways
@ericward-noaa

Copy link
Copy Markdown
Contributor Author

@Schiano-NOAA Thanks for the suggestions, all incorporated here: ac9b513

@Schiano-NOAA

Copy link
Copy Markdown
Collaborator

A1! Thanks so much

@Schiano-NOAA
Schiano-NOAA merged commit 5ca1cfc into nmfs-ost:main Nov 13, 2026
6 checks passed
Schiano-NOAA added a commit that referenced this pull request Nov 13, 2026
* [feat]: convert FIMS output to the standard format in `convert_output()`

convert_output() passed FIMS output through unchanged, so `estimate` and
`year` were empty and no stockplotr plot could use it. The new
conout_fims() reshapes `FIMS::get_estimates()` output (or a FIMSFit object,
or an .rds file with either):

- Each data point (index, landings, age and length compositions) is split
  into "*_observed" and "*_predicted" rows, with the observation error and
  negative log-likelihood on the observed row, as for SS
- `estimated` becomes `estimate`, and the new `start_year` argument converts
  FIMS year indices to years; `era` is "time", or "fore" after the last year
  of data.
- fims_var_names.csv maps current FIMS labels (e.g., expected_recruitment,
  numbers_at_age, log_devs) to the standard labels.

Also fixes:
- convert_output() ignored a user-supplied `model` when `file` is an object.
- plot_index() matched every label containing "index" (e.g., index_log_q in
  Rceattle output or index numbers at age in FIMS output); it now plots only
  index_observed and index_predicted.
- plot_index() failed for a single index fleet, because the fleet facet was
  added back after process_data() dropped the column.

* Update WORDLIST

* [feat]: observation-error intervals and log scale in `plot_index()`

plot_index() adds `interval`, which draws intervals around the observed
index from its observation error, and `log`, which plots the index on a log
scale. Intervals assume lognormal errors, as in Stock Synthesis, FIMS, BAM,
and Rceattle: uncertainty labeled "se", "sd", or "log_sd" is used as the
log-scale standard error, and a CV is converted with sqrt(log(1 + CV^2)).
Both default to the current behavior (no intervals, linear scale).

* [feat]: plot all indices relative to their means with `plot_index(relative = TRUE)`  `relative = TRUE` divides each fleet's observed and predicted index (and intervals) by the mean of its observed index and plots all fleets on one panel, colored by fleet, so that trends can be compared among indices (r4ss SSplotIndices subplot 9). Facet variables with one value for each fleet (e.g., the month of each survey) do not split the panel.

`relative = TRUE` divides each fleet's observed and predicted index (and
intervals) by the mean of its observed index and plots all fleets on one
panel, colored by fleet, so that trends can be compared among indices
(r4ss SSplotIndices subplot 9). Facet variables with one value for each
fleet (e.g., the month of each survey) do not split the panel.

* Update plot_index.R

make sure this is consistent with other outstanding PR

* Update plot_index.Rd

redoc

* Bring same files over to update per commit # 467

* update per changing defaults

* Update R/plot_index.R

Co-authored-by: Sam (Schiano) Bredeck <125507018+Schiano-NOAA@users.noreply.github.com>

* devtools::doc

---------

Co-authored-by: Sam (Schiano) Bredeck <125507018+Schiano-NOAA@users.noreply.github.com>
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.

3 participants