Monolith split#33
Open
taut-and-yare wants to merge 71 commits into
Open
Conversation
Declarations in include/troll.hpp and definitions in src/troll.cpp Updated CMakeLists.txt and .gitignore accordingly Closes #7
…atized-build Add multi-platform build workflow
* Add lint/CMakeLists.txt * Add lint-style and lint-static targets (with `cpplint` and `cppcheck` respectively) * Add workflows/lint.yml * Add lint/README.md Solves #12
* create sanity folder * check script retrieves reference output from main@v4.0 * check script runs current version with reference configuration * check script compares bit by bit reference and current outputs * add cmake sanity and test target * add sanity README --------- Co-authored-by: Dimitri Fekas <dev@yare.fr>
This reverts commit 2d18e762320b29b52a00d7bd28c74e915e870485.
The global parameter registry (AssignParamFromRegistry) has been active since a previous commit. AssignValueGlobal was retained as dead code and is now deleted. Sanity check passes (all 41 output files match reference).
Context ctx is now a file-scope global in troll.cpp instead of a local in main(). RegisterParameters() takes Context& in preparation for binding parameters directly to ctx fields as globals are migrated.
Moves iterperyear, nbiter, iter, nbout, freqout, nbdays, nbsteps_varday, inv_nbsteps_varday, nbhours_covered, timestep from file-scope globals into ctx.time (Context::TimeState). - Adds timestep field to TimeState in context.hpp (was missing) - Removes the ten globals from troll.hpp - Updates all call sites in troll.cpp to use ctx.time.* - Binds nbiter and nbout in RegisterParameters() to ctx.time fields and removes their extern declarations from param_registry.cpp - Preserves output column header string literals unchanged
Moves sites, cols, rows, nbspp, HEIGHT, SBORD, length_dcell, linear_nb_dcells, sites_per_dcell, nbdcells, site_DCELL, i_sites_per_dcell, NV, NH, LV, LH, RMAX, dbhmaxincm, leafdem_resolution from global scope into Context::Grid. - Adds NV, NH, LV, LH, RMAX, dbhmaxincm, leafdem_resolution fields to Grid struct in context.hpp - Removes global definitions from troll.hpp - Removes corresponding extern declarations from param_registry.cpp and rebinds cols, rows, HEIGHT, length_dcell, NV, NH, leafdem_resolution to ctx.grid fields in RegisterParameters() - Fixes one string literal "Number of sites"
Moves all 16 model flag/option globals (_NONRANDOM, _GPPcrown, _BASICTREEFALL, _SEEDTRADEOFF, _NDD, _CROWN_MM, _OUTPUT_extended, _OUTPUT_inventory, _FromInventory, _sapwood, _seedsadditional, _LL_parameterization, _LA_regulation, _OUTPUT_pointcloud, _SOIL_LAYER_WEIGHT, _WATER_RETENTION_CURVE) from global scope into Context::ModelOptions. - Adds _OUTPUT_extended and _OUTPUT_inventory fields missing from the struct in context.hpp - Removes global definitions from troll.hpp - Removes 8 extern declarations from param_registry.cpp and rebinds 11 registry-registered options to ctx.opt fields
…ield Moves all simulation-level input parameters (photosynthesis, allometry, mortality, dispersal) into Context::SimParams (ctx.params), and the two spatial simulation fields (LAI3D, Thurt) into Context::SimFields (ctx.field). Key steps: - Pre-renamed local variables that shadowed the globals before replacement: dens → dens_layer / dens_lowerlayer (crown density locals in 6 functions) alpha → alpha_crown (FvCB canopy integration ratio) alpha → alpha_vgm (van Genuchten-Mualem soil parameter) - Added RENAMED comments at each pre-rename site - Removed all SimParams/SimFields extern declarations from param_registry.cpp - Updated RegisterParameters() bindings to ctx.params.* for all 30 params - Fixed corrupted string literals and character literals: case 'm': (CLI arg parser), parameter_name "g1", column header "g1"
Convert all global definitions in troll.hpp to extern declarations so the header can be included by multiple translation units without ODR violations: - Context ctx, vector<S>, vector<T>, all fstream output_* → extern in header - Definitions moved to troll.cpp alongside the existing file-scope ctx - Add inline to GetParameter (non-template function body in header)
Also fix remaining ODR violations in troll.hpp (mpi_rank, mpi_size, easympi_rank were unconditional definitions; now extern + defined in troll.cpp).
- Move QUAD, CalcVcmaxm, CalcRdark, and all crown geometry helpers (GetRadiusSlope, GetCrownIntarea, GetDensities*, LAI2dens*, UpdateLAI3D, UpdateCHM*, OutputCrownSliced, GetPPFDabove, GetCanopyEnvironment, AddCrownVolumeLayer) out of troll.cpp into src/crown.cpp - Move CircleAreaUpdateCrownStatistic_template and LoopLayerUpdateCrownStatistic_template bodies into troll.hpp so they remain visible at all instantiation sites across future TUs - Replace struct Context forward declaration in troll.hpp with #include "context.hpp" so template bodies can access ctx fields
Move all Tree::* method definitions (~3350 lines) out of troll.cpp into src/tree.cpp. troll.cpp now contains only globals, output stream declarations, and main().
Extracts AssignSpeciesParam, AssignValuePointcloud, ReadInputGeneral, ReadInputSpecies, ReadInputDailyvar, ReadInputClimate, ReadInputSoil (and BuildSpeciesRegistry static helper) — 837 lines — from troll.cpp into src/input.cpp. Adds forward declarations to troll.hpp; input.cpp also needs params/param_registry.hpp for AssignParamFromRegistry.
Extracted InitialiseOutputStreams, Initialise, InitialiseABC, ReadInputPointcloud, ReadInputInventory, AllocMem, FreeMem from troll.cpp into src/memory.cpp (-1178 lines). All forward declarations were already present in troll.hpp.
Extract Average, OutputField, OutputSnapshot, OutputVisual, OutputCHM, OutputLAI, TrackingData_andOutput, all OutputABC*/UpdateABC* helpers, MPI_Share* stubs, and CloseOutputs (~3187 lines) from troll.cpp.
Extracted Evolution, UpdateSeeds, UpdateField, FillSeed, RecruitTree, TriggerTreefall, TriggerTreefallSecondary from troll.cpp (337–1073) into src/simulation.cpp. troll.cpp now contains only main() (~338 lines).
Replace int placeholder fields in OutputConfig with real std::fstream objects; add missing conditional streams (MIP_Lichstein, Output_ABC, WATER, TRACK_INDIVIDUALS). Remove extern fstream declarations from troll.hpp and global definitions from troll.cpp. Update all call sites in troll.cpp, output.cpp, memory.cpp, crown.cpp, and tree.cpp to use ctx.out.*. Move OutputConfig to end of Context to keep hot fields at low struct offsets.
- Species::Init() → Species::Init(Context &ctx) - crown.cpp: GetRadiusSlope, UpdateLAI3D, OutputCrownSliced, GetPPFDabove, GetCanopyEnvironment, AddCrownVolumeLayer all take ctx - Templates: CircleAreaUpdateCrownStatistic_template and LoopLayerUpdateCrownStatistic_template take Context &ctx; GetRadiusLayer promoted to template type param G (enables lambdas) - lookup_tables.cpp: InitialiseIntraspecific, InitialiseLookUpLAImax, InitialiseLookUpTables take ctx - Update all call sites (memory.cpp, tree.cpp, output.cpp, input.cpp) with lambdas where callbacks are involved
- AssignValuePointcloud, ReadInputSpecies, ReadInputDailyvar (×2), ReadInputClimate (×2), ReadInputSoil in src/input.cpp - ReadInputPointcloud, ReadInputInventory in src/memory.cpp - Updated declarations in include/troll.hpp - Updated all call sites in src/memory.cpp and src/troll.cpp
UpdateSeeds, UpdateField, FillSeed (skipped—called from tree.cpp), RecruitTree, TriggerTreefall, TriggerTreefallSecondary, GetTimeofyear. Updated declarations in troll.hpp and all call sites.
Remove extern Context ctx from headers and file-scope definition from
troll.cpp. Declare as local Context ctx{} in main() — value-init
zero-initializes all POD members identically to the old global.
Bug fixed along the way: CloseOutputs() only closed output[0..9],
leaving output[11,21-24,28-33] unclosed. When ctx was global, exit(0)
triggered static destructor cleanup which flushed all fstream buffers
automatically. With ctx as a local variable, exit(0) bypasses local
destructors, so those buffers were silently lost. Fixed by extending
the close loop to cover all 40 slots.
- Create include/species.hpp and include/tree.hpp with class declarations (previously inlined in troll.hpp); move leafFluxes struct to tree.hpp - context.hpp includes species.hpp + tree.hpp and gains vector<Species> S and vector<Tree> T fields in Context - troll.hpp reduced to includes, globals, free-function declarations - Remove file-scope vector<Species> S and vector<Tree> T from troll.cpp - Replace all bare S[]/T[] and S./T. vector-method call sites with ctx.S[]/ctx.T[] across tree.cpp, output.cpp, simulation.cpp, memory.cpp, lookup_tables.cpp, input.cpp
All six functions in the #ifdef Output_ABC block that accessed ctx.* without having ctx in scope now take Context &ctx as their first parameter: OutputABC, OutputABCConservationTraits, OutputABC_ground, OutputABC_species, OutputABC_CHM, OutputABC_transmittance Also fixes a latent bug in OutputABCConservationTraits where gslrng (undefined) was used instead of ctx.rng.gslrand. OutputABCWriteHeaders unchanged — it only writes to the passed fstreams. Call site in troll.cpp updated: OutputABC() → OutputABC(ctx).
Correct typos and improve clarity in REFACTORING.md.
Discuss potential removal of MPI globals
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.
Hello, since we'd left things half-way back in November with @philippeverley , it bugged me and I continued on on this whenever I found some time.
Yesterday I solved a bug that delayed me quite a bit, and the monolith is broken 🎉 ! So it's time for a PR to the dev branch perhaps.
Please read the REFACTORING.md file. I made some splitting/architectural decisions on judjement, but - since I'm an outsider - perhaps some of those are to be revised. Even if so, from now on the codebase should be easier to manage/refactor. There are still some things to do, but I am quite confident I haven't broken anything. See the Confidence section in REFACTORING.md