Skip to content

Refactor BADM_IC_process() for Robust Single/Multi-site Handling#3773

Open
ayushman1210 wants to merge 22 commits intoPecanProject:developfrom
ayushman1210:fix_BADM_C
Open

Refactor BADM_IC_process() for Robust Single/Multi-site Handling#3773
ayushman1210 wants to merge 22 commits intoPecanProject:developfrom
ayushman1210:fix_BADM_C

Conversation

@ayushman1210
Copy link
Contributor

@ayushman1210 ayushman1210 commented Jan 12, 2026

Description

Refactored BADM_IC_process() in modules/data.land/R/IC_BADM_Utilities.R to robustly handle both single-site and multi-site settings with proper coordinate validation.

Changes:

  • BADM_IC_process(): Replaced manual normalization with explicit single/multi-site detection
  • added numeric coercion with validation; improved error handling for invalid coordinates.

Motivation

Code review feedback: improve clarity and robustness of initial condition processing. Original manual handling was prone to coordinate type issues in multi-site workflows.

Types of changes

  • Bug fix

Fixes

#3699

Copy link
Member

@mdietze mdietze left a comment

Choose a reason for hiding this comment

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

Note that most of this PR has to do with unit conversions, not the stated aim of refactoring BADM_IC_process. Recommend splitting into separate PRs

…anProject#3706

- BADM_IC_process now only accepts single Settings objects, not Multisettings
- Validates input has 'run' element; throws clear error if Multisettings attempted
- Multi-site processing should be handled by higher-level wrapper functions
- Updated tests: removed multi-site test, added test for Multisettings rejection
- Uses consistent terminology: 'Settings' instead of 'Settings-like list'
- Aligns with design decision from issue PecanProject#3706 on ic process function interface
@ayushman1210 ayushman1210 requested a review from mdietze January 13, 2026 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants