Skip to content

Fix(#101): Make the catch and discard outputs agree with each other - #103

Open
jacobkasper wants to merge 9 commits into
mainfrom
bug/i101-catch-output-accounting
Open

jacobkasper wants to merge 9 commits into
mainfrom
bug/i101-catch-output-accounting

Conversation

@jacobkasper

Copy link
Copy Markdown
Collaborator

Base: dev ← compare: bug/i101-catch-output-accounting


🌏

Closes #101. Closes #86. Closes #87.

This replaces #88 (fix for #86) and #91 (fix for #87), which I will close in favour of this PR, and adds the other output problems collected in #101. They were found while validating the detailed catch output in #32. That output turned out not to be needed, since OutCATCH.nc already gives catch by species, fishery and box, so #32 will be closed separately. The recreational catch outputs have the same kinds of problems but can't be tested here, so they are in their own issue, #99, and not changed in this PR.

Justification

Atlantis writes the same catch and discards to several outputs: OutCatch.txt, OutCatchPerFishery.txt, OutDiscard.txt, OutDiscardPerFishery.txt, OutCATCH.nc and OutTOTCATCH.nc. Summed over their extra dimensions they should agree, and anyone analysing catch by fishery, or discards from the NetCDF files, depends on that. They didn't. This PR fixes the causes, one commit each:

# Problem (see #101 for details) Fix
1 OutCatchPerFishery.txt about 0.15 % low every year: the year's last step is zeroed in bm->Catch before it reaches TotCumCatch (#86) Add the step still in the buffer to the completed year before the annual report (from #88)
2 NetCDF discards (Tot_<sp>_Discards, <sp>_Discard_FC<n>) inflated, about 1.44 × in Icelantis and 4 × in SETas: box-level discards added once per layer (#87) Add them once per box (from #91, without its rec-catch part, which moves to #99)
3 OutDiscardPerFishery.txt: each year's last step reported in the following year Correct the written values; TotCumDiscards itself is unchanged
4 Last row of OutCatchPerFishery.txt repeats the previous year at the end of a run Report the current year at the end-of-run write, including the step still in the buffer
5 Avg_Catch_Sze_* carry the effort variable's long name Give them their own long name
6 Catch and discard variables in OutCATCH.nc / OutTOTCATCH.nc are named by group Code, unlike Out.nc and the age-class catch variables, which use the Name Use the Name, e.g. Tot_FCD_Catch → Tot_Cod_Catch, FCD_Catch_FC7 → Cod_Catch_FC7

Results. dev (4fe7347) against this branch. Each figure is the output summed over boxes and fisheries, divided by OutCatch.txt or OutDiscard.txt:

Output SETas, dev SETas, branch Icelantis, dev Icelantis, branch
OutCatchPerFishery.txt 0.9986 1 0.9980–0.9988 1
…its last row (run ending mid-year) repeats previous year = OutCatch.txt repeats previous year = OutCatch.txt
NetCDF discards, water-column group 3.97–4.02 1 1.442–1.447 1
NetCDF discards, epibenthic group 1 1 – –
OutDiscardPerFishery.txt 0.9986, then 0.9999–1.0001 1 0.9985, then 0.9995–1.0008 1
NetCDF catch 1 1 1 1

SETas: 1,100-day runs. Standard SETas has no discards, so the discard rows come from a run with discarding switched on for two groups (see reviewer instructions). Icelantis: 10-year runs. In Icelantis, Tot_<sp>_Catch also equals the sum of <sp>_Catch_FC<n> box by box (difference 0 t).

Everything else is unchanged. Comparing every output file between dev and the branch:

  • Icelantis: 28 of 31 outputs are identical, including Out.nc, the biomass, mortality, SSB, catch, discard, TAC and effort outputs. The three that differ are the intended ones: OutCatchPerFishery.txt, OutDiscardPerFishery.txt and OutHarvestIndx.txt.
  • SETas: the same; all NetCDF values are identical apart from the renamed and corrected catch variables.

Things reviewers should know:

  • Fix 1 is not purely an output change. The added step also goes into TotCumCatch, CumCatch, TripCatch and TotCumBiCatch, which management, quota, assessment and economics code read. Biomass is unchanged in both test models, but in a model whose catch-based management reads these during the annual block, results could change slightly. That would be a correction: the step was simply lost before. OutHarvestIndx.txt changes for this reason (TotLanding up by about one timestep's catch per year).
  • Fix 3 deliberately corrects only the written file. TotCumDiscards is also used by the quota and management code, so changing its year attribution would change management results. That is left for a separate discussion.
  • Fix 6 is a breaking change for scripts that read these NetCDF variables by name. It broke my own check script, which now maps Name to Code using the groups CSV.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other change (if none of the other choices apply)

Items 1–5 are bug fixes. Item 6 renames output variables.

Further comments

  • I've created an issue
  • This is NA

AtlantisManual issue: #, to document the renamed variables in OutCATCH.nc and OutTOTCATCH.nc, and that the per-fishery outputs now match the totals.

Reviewer instructions:

Suggested reviewers: a maintainer, plus Andy, who reviewed #88 and #91.

1. Build dev and this branch, with your usual build (Docker or autotools):

git fetch origin
git worktree add ../at_dev origin/dev
git worktree add ../at_i101 origin/bug/i101-catch-output-accounting
for d in ../at_dev ../at_i101; do
  (cd $d/atlantis && aclocal && autoheader && autoconf && automake -a && ./configure && make)
done

2. Run SETas for 1,100 days with each build. Set this in VMPA_setas_run_fishing_F_Trunk.prm in a copy of example/:

tstop     1100 day
toutinc   365 day
toutfinc  365 day

Run with the command line in example/RunAtlantis.sh. Each run takes about a minute.

3. Optional, to test the discard fixes (2 and 3). Standard SETas has no discards. In a second copy of example/, edit VMPA_setas_harvest_F_Trunk.prm:

  • FFCDR_FPS: set the 3rd value (midwcFP) to 0.1;
  • FFCDR_BFS: set the 4th value (dredgeBFS) to 0.1;
  • incidmort_FPS: set the 3rd value to 1.0;
  • add an incidmort_BFS line (33 values, 1.0 in 4th place, 0.0 elsewhere).

FPS tests the water-column path and BFS the epibenthic path.

4. Compare with the attached R scripts:

  • check_catch_nc.R compares every catch and discard output with OutCatch.txt and OutDiscard.txt by year. For the branch, set groups_file to SETasGroupsDem.csv, since the NetCDF variables now use the group Name.
  • compare_opt_levels.R compares two runs file by file. With base_dir = the dev run and other_dirs = the branch run, only OutCatchPerFishery.txt, OutDiscardPerFishery.txt and OutHarvestIndx.txt should differ, plus the discard variables in the two catch NetCDF files.

Expected on the branch:

  • all ratios are 1;
  • the last row of OutCatchPerFishery.txt equals the final partial year of OutCatch.txt;
  • all other outputs are identical to dev.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CcJCYq31xJF2pEZfr66D3J

andybeet and others added 9 commits November 5, 2026 09:58
…-link

Chore(#98): Correct link to contributing.md
Harvest_Annual_Reset zeroes the per-step catch buffer bm->Catch
before the year's final step has been drained into TotCumCatch and
CumCatch. OutCatchPerFishery.txt therefore missed one timestep of
catch every year (about 0.15 %), while OutCatch.txt and the NetCDF
catch tracers, which are added to in the step the catch is taken,
were correct.

Add the step still in the buffer to the completed year just before
the annual report. The buffer is not zeroed here: the annual reset
still does that, so other code that reads bm->Catch is unaffected.

Part of #101 (was #86).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CcJCYq31xJF2pEZfr66D3J
Harvest_Update_Total_Catch runs once per water-column layer. Catch is
stored per layer, so adding it per layer is correct, but bm->Discards
and bm->totdiscards are box totals with no layer index, so the same
discards were added at every layer. Tot_<Code>_Discards and
<Code>_Discard_FC<n> were about 1.44 times OutDiscard.txt for FHA.

Add the discard tracers once per box: at layer 0, the last layer
processed, for water-column groups, and in the EPIFAUNA call for
epibenthic invertebrates. The recreational catch tracer has the same
problem but is left unchanged here; it is covered by #99.

Part of #101 (was #87).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CcJCYq31xJF2pEZfr66D3J
bm->Discards is drained into TotCumDiscards at the start of the next
step, but unlike bm->Catch it is not zeroed at the end of the year.
The last step of each year therefore reached TotCumDiscards after the
annual reset, and OutDiscardPerFishery.txt reported it in the following
year. Run totals were right; yearly values were off by one boundary
step, in either direction.

Correct the output only: when writing OutDiscardPerFishery.txt, add
the discards still in the buffer and subtract those carried in from
the previous report. TotCumDiscards itself is unchanged, because the
management, quota and assessment code also reads it.

Part of #101.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CcJCYq31xJF2pEZfr66D3J
writeAnnCatchPerFishery read TotCumCatch[...][thisyear - 1], which is
the year just completed when called from the annual report, after
thisyear has been advanced. At the end of a run Textfile_Dump calls
the same writer before thisyear advances, so the last row of
OutCatchPerFishery.txt repeated the previous year.

Pass the year to report to Harvest_Report_Annual_Stats: the year just
completed for the annual report, the current year when the run ends.
At the end of the run also add the catch still in the per-step buffer
to the current year, so the final partial year matches OutCatch.txt.
Textfile_Dump is only called as the program stops, so this cannot
count a step twice.

Part of #101.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CcJCYq31xJF2pEZfr66D3J
The Avg_Catch_Sze_FC<n> and Avg_Catch_Sze_Recfish variables in
OutTOTCATCH.nc were given the long name of the effort variables
("Total effort of fishery ..."), as a comment in the code noted.
Name them "Average catch size of ..." instead. Only the long_name
attribute changes; variable names and values are unchanged.

Part of #101.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CcJCYq31xJF2pEZfr66D3J
Out.nc and the age-class variables in OutCATCH.nc are named by the
group Name (Cod1_Nums, Cod1_Catch), but the total and per-fishery
catch, discard and rec-catch variables used the group Code
(Tot_FCD_Catch, FCD_Catch_FC7). Use the Name for these too
(Tot_Cod_Catch, Cod_Catch_FC7), and the full name in the per-fishery
long names, as the Tot_ variables already do.

This renames existing variables in OutCATCH.nc and OutTOTCATCH.nc,
so scripts that read them by name need updating. Values are
unchanged.

Part of #101.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CcJCYq31xJF2pEZfr66D3J
Part of #101.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CcJCYq31xJF2pEZfr66D3J
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants