The big Sync. - #254
Open
odav wants to merge 1315 commits into
Open
Conversation
Fixed bmix not being output for csv type files when outputting hru_carbvars.csv.
Signed-off-by: fgeter <110357015+fgeter@users.noreply.github.com>
Bulk changes for carb vars, decomp, tillage, many fixes
Minor fixes.
…in n, c, and p amounts.
Changes to correct cbn_zhang2 and added the tracking of lignin and nonlignin and updated the code to output the same in appropriate output files.
…hru_soilcarb_mb_stat
… out when cbn_diagnosits == 1
Update pl_manure.f90 with many corrections
Corrected mgt_biomass to correctly reflect the potentional bio mixing for each day. Changed a header column in hru_carbvars frin bmis to cons_bmix.
Fixed issue of tillagef not being initialized to 0. in cbn_zhang2.
…notebook code. Removed hru_rsdc graphs from jupyter notebook.
…ing in regard to writing headers one time for hru_cbn_lyrs and hru_seq_lyrs
…rite is being called.
Update to cbn_zhang2 to increase cs maximum, cleaned soil_nutcarb_write and command and updated dockerfile for codespaces.
tillage and manure fixes.
…a previous commit.
9082bf8 took CSU_main's CMakeLists.txt wholesale, which dropped upstream rel 62's macOS else() branch. Frank asked for that block back: it statically links the gfortran/quadmath/gcc runtimes so a Mac binary does not depend on Homebrew's gcc dylibs, and without it macOS users need `brew install gcc`. Block is byte-identical to rel 62's. It sits inside `if(NOT APPLE) ... else()` so it is inert on Linux -- gfortran flags unchanged, configure emits the same FFLAGS, and RUN 1-6 remain valid. NOTE: CMakeLists.txt is therefore no longer identical to CSU_main's; this one block is the only difference. Apply the same block to CSU_main to re-converge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adopts upstream rel 62's macOS else() branch, which statically links the gfortran/quadmath/gcc runtimes so a Mac binary does not depend on Homebrew's gcc dylibs (libgfortran.5.dylib, libquadmath.0.dylib). Without it, macOS users need `brew install gcc` to run the executable. Block is byte-identical to rel 62's and to the one restored on staging_merge in 0e1b609, so CMakeLists.txt is identical across main and staging_merge again. Inert on Linux -- it sits inside `if(NOT APPLE) ... else()`, gfortran flags are unchanged, and configure emits the same FFLAGS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Team decision (2026-08-19): carbon has two modes, 0 = static and 1 = CENTURY.
This reverses cleanup item C1 and standing rule R2, which had accepted the
interim numbering (1 reserved for C-FARM, 2 = CENTURY) inherited from
swatplus rel 62. The new numbering matches CSU_main / legacy SWAT again.
Code (20 files):
- 36 `cswat == 2` -> `cswat == 1`
- carbon_bsn_read.f90: C-FARM case removed; CENTURY is now case (1);
carbon = 2 gets a dedicated migration error, not the generic one
- soil_nutcarb_write{,_historical}.f90: `case (2)` -> `case (1)`
- basin_module.f90: declaration comment and the stale legacy enumeration
- flush(6)/flush(9001) before error stop, plus an error-stop message --
without them the buffered text was discarded and users saw only a bare
ERROR STOP and a backtrace
mgt_newtillmix_cswat1/cswat0 keep their names: they were named for the
legacy convention and are correct again under it.
Data: carbon 2 -> 1 in 30 fixtures (refdata/ tracked here; workdata/ is
gitignored and travels via unison).
Verified numerics-neutral: Raccoon 107 HRU 1997-1998, all 12 objects
yearly+avann. New build at carbon = 1 is byte-identical to the pre-change
build at carbon = 2 across all 39 outputs. cswat = 0 unaffected (39 outputs,
clean run). carbon = 2 now aborts with the migration message on stdout and in
diagnostics.out. The CSU_main vs staging_merge baseline still reads
9 identical / 30 differ / 10 CSU_main-only -- and both sides now use
carbon = 1, so the comparison no longer needs differing codes.bsn.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings the fork's release-61 line up to upstream SWAT+ release 62 while keeping CSU_main's own science. 1129 commits, developed and validated on staging_merge per tmp/merge_decisions.md. Conflicts: 2 files, one logical cause. Both sides had independently fixed the duplicate-carbon-row bug -- CSU_main in 47a265e/b92b7fc, staging_merge in fbd2aa1 -- against different carbon-output architectures: src/command.f90 CSU_main gated the whole-basin writers on the single legacy pco%cb_hru flag; staging_merge gates on the six per-family flags (cb_lyr/cpool/npool/plt/flux/snap_hru) AND keeps the legacy path via soil_nutcarb_write_historical. src/hru_output.f90 comment-only; same statement worded per architecture. Both resolved toward staging_merge. Its version is a superset -- new per-family gating plus the legacy writer -- and it is the tree that rules R4/R5 specified and that every validation run exercised. Taking CSU_main's side would have reverted the carbon output architecture this merge exists to adopt. Result: the merged tree is byte-identical to staging_merge. Verified by build and by regression -- Raccoon 107 HRU 1997-1998, all 12 objects yearly+avann, 39 outputs identical to staging_merge, 0 differ. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The selective rel 62 merge left process annotations in the code: cleanup-item
tags (C2, C4, C5, C7, C12), rule tags (R1), branch names from the merge
(staging, fg), dated attributions, and references to tmp/merge_decisions.md
items. Those tracked the merge; they mean nothing to someone reading the code.
Removed the bookkeeping, kept every technical rationale. Two examples:
cbn_surfrsd_decomp.f90 keeps WHY the denormal guard exists (abg_rsd decays
asymptotically and gfortran raises underflow); drops
the "(C4)" tag and the re-enable history.
carbon_module.f90 keeps WHY n_act_frac is unread (cswat==0 only, and
carbon_bsn_read returns early on that path); drops
"merge decision (c)" and "post-merge item C12".
One comment block deleted outright: the (C5) note in cbn_surfrsd_decomp.f90
described dead code that had already been removed, so it documented nothing.
Left alone: `!dont merge` in nut_solp.f90 -- it came from the 2024 bitbucket
import, not from this merge, and may still matter for upstream syncs.
Comments only. Code lines are byte-identical after stripping trailing
comments, and Raccoon (107 HRU, all 12 objects) gives 39/39 identical output.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sets carbon.bsn's last column (cbn_diagnostics) from 1 to 0 for the Ames_sub1 reference dataset, suppressing the legacy carbon diagnostic output written from carbon_legacy_module. Data only; no code change. Other fixtures still have it enabled -- notably refdata/IA-Clayton_Test_Case -- so this is Ames_sub1 specific, not a project-wide default change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SWAT+ writes its output into whichever directory it runs in, so running a reference dataset in place litters refdata/ with .txt/.csv/.out files. That was handled by 44 hand-listed /refdata/Osu_1hru/*.txt entries -- which covered one subfolder and needed a new line per output file. Replaced with three patterns covering every subfolder at any depth: refdata/**/*.txt refdata/**/*.csv refdata/**/*.out Safe: refdata/ holds INPUTS ONLY. All 238 tracked files there are SWAT+ input extensions (bsn, sol, cli, con, ops, ini, ...) and not one is a .txt/.csv/.out, so no tracked file is shadowed -- verified with `git ls-files refdata/ | git check-ignore --stdin`, which returns nothing. Also ignores refdata/clean_files.sh. That helper was never tracked; its canonical copy lives in workdata/ (gitignored) and the refdata duplicate has been removed from both machines. Net: .gitignore 122 -> 91 lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fixture's print.prt line 11 named the object `bsn_wb`. The only valid name
is `basin_wb` -- `bsn_wb` appears nowhere in src/ -- so basin_print_codes_read
aborted the run:
Error: The output object bsn_wb in the input file print.prt is not a valid
object. and cannot be processed
This fixture has therefore never run. Confirmed pre-existing, not a regression:
a build of bc6e4f8 (CSU_main immediately before the rel 62 merge) fails
identically, and the strict object-name check is present in the merge base
41b2b9d as well.
After the fix the fixture runs clean: exit 0, 22 outputs, no NaN/Inf.
Column alignment preserved (basin_wb is 2 chars longer, so 2 spaces removed).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
parameters.bsn field 43 held '0.00000'. basin_read_prm.f90:24 reads the whole bsn_prm derived type positionally, and member 43 is `integer :: day_lag_mx`. gfortran rejects a decimal token into an INTEGER, so the read aborts there -- and the routine only tests `if (eof < 0)`, i.e. EOF, so a positive iostat from a type error is never noticed. Fields 43-44 silently keep compile-time defaults, leaving igen at 5 instead of the file's 0. That is not cosmetic. gcycl.f90:82 tests `if (bsn_prm%igen /= 0)`, so the defect switches the weather generator on and reseeds it with 100*igen*rn. Measured on IA-Ames_ResidueHarvest: correcting the token changes 21 of 26 output files -- water balance, carbon pools and yields. Both variants are deterministic (verified 26/26 run-to-run), so this was silently wrong rather than unstable. Fixed here: refdata/IA-Clayton_Test_Case and refdata/Osu_1hru. The same fix was applied to 7 workdata fixtures (gitignored, so they travel by unison): CA-Stanislaus_EoceneDC2, IA-Ames_ResidueHarvest, IA-Ames_sp40_Clarion, IA-Ames_sp40_Clarion_manu, IA-Harrison_McPaul, IA-Harrison_sp40_v64-1979, nutcarb_init_issue. Deliberately NOT fixed: workdata/IA-Clayton_Test_Case_Input_Errors (field 43 = '1.0'). That fixture exists to carry input defects. Found by test/check_inputs.py, which was written for exactly this bug class. Column widths preserved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merge of rel 62 code base with CSU codebase
Local work uses many parallel build directories (build-rel, build-omp, build-ubuntu-ifx-omp, build-arch-static, ...) and only the bare build/ was ignored, so 18 of them showed up as untracked noise in git status. Widen the pattern to build*/. The trailing slash restricts it to directories, so tracked files whose names start with "build" (.github/workflows/build.yml, doc/Building.md, doc/images/BuildAll.png, doc/images/build_all_files.png) are unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… in the previous merge with rel 62 Added a new input file called soil_test.sol. It will be read only if cswat == 1 and only if the file exists. Modified proc_read to read the file and added a new subroutine to acturally read the new input file.
nut_np_flow computes, per pool-to-pool flow, the N released to the mineral pool (MNR_AtoB). Part of that is the nitrogen carried by the carbon respired as CO2, held in EfromCO2 and computed correctly at line 37. Line 61 then added EFCO2 instead - a second local, declared with the descriptive comment and never assigned anywhere in the program. Fortran is case-insensitive, so EFCO2 bound to that always-zero variable and every CO2-associated N mineralization in the CENTURY model was silently discarded. The CO2 flow out of a pool is routinely 3-4x the transfer flow, so the loss is large. cbn_zhang2 keeps two independent accounts of net mineralization for the same layer-day: rnmn = sum - trnn (EPIC-style, aggregated by source pool) credits the mineral pool, while SUM(mnr*) - SUM(imm*) (per-flow, from nut_np_flow) debits the organic pools. They are algebraically identical - both reduce to (N leaving pool A) - (N required by pool B) - but this bug made the per-flow account 33x smaller. Splitting them into supply and demand confirms it: the demand sides agree, and the whole gap is on the supply side. While rnmn was inert the discrepancy only under-debited the organic pools. 5aa6ab1 correctly made rnmn credit the mineral pool as ammonification, and the gap became a nitrogen source of ~300 kg N/ha/yr - more than a full fertilizer rate, which is why unfertilized cswat=1 runs showed no N stress. Validated on workdata/PC-till+40-Adair (1 HRU, 35 yr, carbon=1, no fertilizer in management.sch), measured with per-layer per-day N-conservation checkpoints: before after N created, 34 yr +13068.6 -142.8 kg N/ha soil NO3 at end 3982 29 kg N/ha mean strsn 0.0033 0.2725 soybean fixation 44 1683 kg N/ha corn yield 8748 613 kg/ha soybean yield 3459 3455 kg/ha soil C change +45.4% +13.0% 99.9% of the imbalance is gone. The fixed run behaves as an unfertilized system should: nitrate returns to a normal 29 kg N/ha, real N stress appears, soybean fixation reaches a realistic 1683 kg N/ha (independent cswat=0 control: 1659), corn collapses, and soybean is unchanged because it fixes its own N. swat-c is correct here - SWAT_EFLOW_new.f90:65 uses EfromCO2, and EFCO2 appears there only in a comment. The defect entered when that routine was ported to SWAT+: the comment's name became a variable declaration. nut_np_flow.f90 is byte-identical on this fork's main, on bc6e4f8, and on upstream/main, and has been present since df07e3f (2024-03-05). This is a shared upstream bug and should be reported. The now-unused efco2 declaration is removed so the mistake cannot recur; its comment moves to EfromCO2, which is the variable it actually described. CALIBRATION-BREAKING for every cswat=1 run. Two caveats before relying on fixed-model output: post-fix net mineralization is ~12.6 kg N/ha/yr, which looks low for a soil holding 8000+ kg N/ha of organic N (corn at 613 kg/ha is probably too low), and a separate unfixed inconsistency remains - the s1->s3 flow in cbn_zhang2.f90 passes the slow-humus C:N where the destination is passive humus (worth ~36 kg N/ha over 34 yr). Full analysis in tmp/century_n_creation_defect.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The microbial biomass -> passive humus transformation passes
1/org_ratio%nchs - the SLOW humus C:N - to nut_np_flow as the receiving
pool's ratio. The receiving pool is s3, passive humus, so it should be
1/org_ratio%nchp.
Three things in the same file say so:
- the destination is passive humus: the comment says so, and the C flow
cfs1s3 = bmcta * abp uses abp, the biomass->passive allocation fraction;
- cbn_zhang2 computes the same N demand a second way, and both places it
appears (lines 540 and 650) use nchp:
pn5 = bmcta * org_allo(cf_lyr)%abp * org_ratio%nchp !biomass to passive
- the sibling s2->s3 call, also into passive humus, correctly passes
1/org_ratio%nchp.
nut_np_flow derives the receiving pool's N demand as C_AtoB / CEtoB. Because
nchs < nchp in every branch of the ratio calculation, the demand is
understated - by 33% at ncbm=.1, 42% at layer 1, 60% at ncbm=.05 - so the
flow immobilizes too little and mineralizes too much.
The decisive evidence is that the model's two independent accounts of the
same transformations reconcile only with nchp. cbn_zhang2 keeps an
EPIC-style aggregate (sum/trnn, which credits the mineral pool) alongside a
per-flow account from nut_np_flow (which debits the organic pools).
Instrumenting both per layer-day and splitting each into supply and demand,
the microbial pool was the ONLY pool whose demand sides disagreed:
pool EPIC demand per-flow demand gap
structural litter 927.3 927.3 0.0
metabolic litter 119.2 119.2 0.0
microbial (s1) 388.4 378.6 9.8
slow humus (s2) 164.7 164.7 0.0
passive humus (s3) 75.4 75.4 0.0
This change closes it exactly: 388.5 vs 388.5, gap 0.0.
Note the limit of that argument: it proves nchs is inconsistent with the
rest of the model, not that nchp is the physically correct stoichiometry.
Anyone wanting nchs here would have to change pn5 to match.
Validated on workdata/PC-till+40-Adair (1 HRU, 35 yr, carbon=1, no
fertilizer), measured on top of a334ede so the EFCO2 defect would not mask
the result:
before after
microbial demand gap 9.8 0.0
N created, 34 yr -142.8 -133.1 kg N/ha
soil NO3 at end 29 29 kg N/ha
mean strsn 0.2725 0.2724
soybean fixation 1683 1683 kg N/ha
corn yield 613 614 kg/ha
soybean yield 3455 3455 kg/ha
The N-balance improvement (9.7) matches the demand gap it closes (9.8), and
model outputs are essentially unchanged. Unlike a334ede this is not
calibration-breaking; it removes an internal inconsistency.
Unlike the EFCO2 defect, which SWAT+ introduced while porting swat-c's
SWAT_EFLOW_new.f90, this one is inherited: swat-c passes 1./NCHS at
carbon2_new.f90:1220 and is internally inconsistent the same way (its
DF5 = HPNTA - NCHP*X1 uses NCHP for the passive pool). Present identically
on this fork's main, on bc6e4f8, and on upstream/main - worth reporting to
both, framed as an inconsistency with pn5 rather than a settled
stoichiometry question.
Full analysis in tmp/s1s3_nchp_defect.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Structural litter decomposes along two paths - str -> s1 (microbial biomass,
non-lignin carbon) and str -> s2 (slow humus, lignin carbon) - and part of
its carbon is respired as CO2 on each. nut_np_flow needs the CO2 belonging
to THAT flow, because it converts it into the nitrogen carried out of the
pool by the respired carbon (EfromCO2 = E_A * CO2fromA / C_A) and adds it to
mineralization.
swat-c keeps the two shares as separate array elements:
CO2FSTR(k,LIG,j) = ALSLCO2 * LSLCTA(k,j) ! -> str to s2
CO2FSTR(k,NONLIG,j) = ALSLNCO2 * LSLnCTA(k,j) ! -> str to s1
SWAT+ collapsed them into the single scalar org_flux%co2fstr holding their
SUM, and passed that total to BOTH calls. The nitrogen carried by respired
structural carbon was therefore counted twice - once per flow.
Each call now receives only its own component. org_flux%co2fstr keeps the
total because it is reported as a C flux (soil_nutcarb_write.f90:559). The
shares are passed as expressions rather than new locals: CO2fromA is
intent(in), lslcta and lslncta are not modified between the co2fstr
assignment and the calls, and this avoids adding saved locals (an
initializer implies SAVE, a shared-state hazard under threading - see
tmp/threading_playbook.md Part 20).
This was latent until a334ede. Before it, EfromCO2 was discarded entirely
because nut_np_flow added a never-assigned EFCO2 instead, so the duplicate
argument had no effect. Fixing that made the double count live.
Validated on workdata/PC-till+40-Adair (1 HRU, 35 yr, carbon=1, no
fertilizer), measured on top of a334ede for that reason, with per-layer
per-day N-conservation checkpoints and a per-source-pool supply/demand
split:
before after
structural supply gap -131.1 0.0
structural demand gap 0.0 0.0
N created, 34 yr -142.8 -12.5 kg N/ha
soil NO3 at end 29 15 kg N/ha
mean strsn 0.2725 0.2571
soybean fixation 1683 1659 kg N/ha
corn yield 613 863 kg/ha
soybean yield 3455 3443 kg/ha
soil C change +13.0% +13.6%
The N-balance improvement (130.3) matches the supply gap it closes (131.1).
Nitrate and soybean fixation land exactly on the values the independent
cswat=0 carbon model produces (15 and 1659), which is corroboration from a
separate code path.
Combined with 37c659d (s1->s3 passive-humus C:N), nitrogen created over 34
years falls to -1.7 kg N/ha - single-precision roundoff across ~171,000
layer-days - and every pool's supply AND demand reconcile to 0.0.
This one does move model output (corn +250 kg/ha here), so it is
calibration-relevant, unlike 37c659d.
nut_np_flow.f90 and cbn_zhang2.f90 are byte-identical to upstream/main, so
upstream has this too and it should be reported alongside a334ede.
Full analysis in tmp/century_n_creation_defect.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fix three defects that let the CENTURY (cswat=1) soil model create nitrogen
The notebooks are stored stripped (no outputs, no execution counts, no language_info, sequential cell IDs) via an nbstripout clean filter. The .gitattributes entries are committed but the filter definition is per-clone local git config, so a fresh clone silently commits raw notebooks with randomized cell IDs and reintroduces merge conflicts. Note that `nbstripout --install` discards `--extra-keys`; the filter.nbstripout.extrakeys config setting is required separately to strip metadata.language_info. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Restores the ability to read measured soil test results, which was dropped in the rel-62 merge. Adds src/soil_test_read.f90 and its call site in proc_read, after soil_db_read so the test soil names can be cross-checked against soildb. soil_test.sol is read only when codes.bsn carbon = 1 and only when the file exists; it is discovered by inquire and has no file.cio entry, the same as carbon_lyr.bsn. When absent, sol_test is left unallocated and soils_init skips soils_test_adjust, so the run is unchanged. refdata/Ames_sub1/soil_test.sol merged as a no-op -- main already carried byte-identical content from 1e195d7. .claude/settings.local.json, committed on the branch in cd2fbda, is dropped here: it is per-clone Claude Code permissions carrying a hardcoded /home/fgeter path and does not belong on main. Verified: builds clean under the gfortran_release_linux preset (CMake globs src/*.f90, so no CMakeLists change). A 2-year Ames_sub1 run confirms the reader fires and that checker.out changes for exactly the two soils named in soil_test.sol. This CHANGES Ames_sub1 output -- 21 of 88 output files differ against a run with soil_test.sol removed -- because that dataset runs carbon = 1 and now has the file present. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cbn column held 4.0/3.0/3.0/3.0 %OC, which is high for cultivated
central-Iowa Mollisols -- 4.0 %OC is roughly 6.9% organic matter, at the top
of the native-prairie range rather than a plausible measured value for
cropped ground.
Replaced with values consistent with each soil's drainage class and with the
soils.sol baseline they override:
soil_01-h1 (well drained) 0-150 mm 4.0 -> 2.10 (soils.sol 0-50: 1.98)
150-250 mm 3.0 -> 1.75 (soils.sol 50-150: 1.33)
soil_03-h3 (poorly drained) 0-125 mm 3.0 -> 3.40 (soils.sol 0-50: 4.20)
125-150 mm 3.0 -> 3.00 (soils.sol 50-150: 2.85)
Only the cbn field changed; name, depth, bd and the texture fractions are
byte-identical and sand+silt+clay still totals 100.0 on every row.
Effect on a 2-year Ames_sub1 run, against the same deck with soil_test.sol
removed: basin soil C at end of 1976 was 404,009 kg/ha, now 391,440 against a
380,310 baseline -- the overstatement drops from +6.2% to +2.9%. The same 21
of 88 output files still differ, as expected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Restore soil test input (soil_test.sol), fix a carbon.bsn error message, document nbstripout setup
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is it.