Skip to content

Conversation

@sunt05
Copy link

@sunt05 sunt05 commented Nov 15, 2025

Add comprehensive validation for physics method configurations to enforce critical compatibility rules between methods. Includes:

  • EHC storage heat requires SPARTACUS net radiation (≥1000)
  • SPARTACUS requires EHC storage heat pairing
  • STEBBS storage heat requires active stebbsmethod
  • RC method requires active STEBBS
  • OhmIncQf inclusion requires OHM-based storage methods

Validation includes explicit None checks, improved error formatting with numbered lists for multiple errors, and full test coverage. Documentation enhanced with depends_on/provides_to metadata and proper cross-reference links.

@sunt05 sunt05 temporarily deployed to github-pages-preview November 15, 2025 00:48 — with GitHub Actions Inactive
@sunt05 sunt05 changed the title Add physics method dependency validation Add physics method dependency description in docs Nov 15, 2025
@github-actions
Copy link

github-actions bot commented Nov 15, 2025

🔍 Schema Preview Deployed

Preview URLs:

Production URLs (unchanged):


⚠️ Important: Preview schemas are in a subdirectory and do not affect production. The preview pages include warning banners to prevent accidental use in production configs.

@github-actions
Copy link

🤖 I've automatically formatted the code in this PR using:

  • Python: ruff v0.8.6
  • Fortran: fprettify v0.3.7

Please pull the latest changes before making further edits.

@sunt05 sunt05 temporarily deployed to github-pages-preview November 15, 2025 00:51 — with GitHub Actions Inactive
sunt05 and others added 4 commits November 15, 2025 09:09
…ndling

Add comprehensive validation for ModelPhysics configuration to enforce
critical compatibility rules between physics methods:
- EHC storage heat requires SPARTACUS net radiation (>= 1000)
- SPARTACUS net radiation requires EHC storage heat
- STEBBS storage heat requires active stebbsmethod
- RC method requires active STEBBS
- OhmIncQf inclusion requires OHM-based storage heat methods

Improvements:
- Add type hints to _unwrap_method_value() helper function
- Add explicit None checks to prevent TypeErrors
- Format multiple errors as numbered list for clarity
- Add comprehensive test coverage for all validation rules
- Enhance documentation with dependency metadata (depends_on, provides_to)
- Add clarifying comments for physics_anchor_fields in RST generator
Add blank lines after depends_on and provides_to in RST generation to ensure
they appear on separate rows in the rendered documentation instead of on the
same line. This improves readability of method interaction metadata.
Co-authored-by: sunt05 <1802656+sunt05@users.noreply.github.com>
Replace complex sys.path manipulation and mock module creation with standard
import pattern used by other tests in test/data_model/. Pytest handles import
paths correctly without this boilerplate.
json_schema_extra={"unit": "dimensionless"},
json_schema_extra={
"unit": "dimensionless",
"depends_on": ["ohmincqf", "snowuse"],
Copy link
Author

Choose a reason for hiding this comment

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

Hi @dayantur - this shows the idea I mentioned about using data structure to illustrate the module dependency.

Copy link

Choose a reason for hiding this comment

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

Hi @sunt05, I had a quick look and it seems like a good improvement.. definitely a faster way to implement this kind of rule. I think we just need to make sure we clean up the existing dependency rules in phase_b.py (there are already a couple or so in there), and that we place this new model validator in SUEWSConfig (since we've moved all validators into config.py - unless you think this specific case requires otherwise?).

I’m about to push a couple of updated CSV files in PR #851, where I’ve collected all the rules currently in the pipeline. That should give us a clearer picture of what would need to be deduplicated if we move forward with this approach.

P.S.: I think we can agree - though correct me if you think I'm wrong! - that we'll still need some rules in phase_b.py for physics-option dependencies when a particular combination of model physics settings imposes checks or constraints on other parameters, rather than just encoding the basic physics-option dependency itself.

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