Skip to content

fix: update constants precision and centralize definitions#84

Closed
Copilot wants to merge 5 commits into
developfrom
copilot/swmm6-rel-constants-precision-nits
Closed

fix: update constants precision and centralize definitions#84
Copilot wants to merge 5 commits into
developfrom
copilot/swmm6-rel-constants-precision-nits

Conversation

Copy link
Copy Markdown

Copilot AI commented May 28, 2026

Several physical constants were specified at reduced precision and duplicated across files. GRAVITY was 32.2 (should be 32.174), SI_GRAVITY was 9.81 (should be 9.80665), and PHI was 1.486 (should be 1.4859, i.e. 0.3048^(-1/3)). Hardcoded 1.49 appeared in multiple source files instead of referencing the named constant.

Precision updates

Constant Old New
GRAVITY 32.2 32.174
SI_GRAVITY 9.81 9.80665
PHI 1.486 1.4859

Applied consistently to both:

  • src/legacy/engine/consts.h
  • src/engine/core/Constants.hpp (+ derived SQRT_GRAVITY, INV_SQRT_GRAVITY)

Centralized constant usage (refactored engine)

  • Removed duplicate local PHI definitions in Transect.cpp, Runoff.hpp
  • Removed duplicate local GRAVITY in HydStructures.hpp, XSection.cpp
  • Replaced hardcoded 32.2 in ForceMain.cppconstants::GRAVITY
  • Replaced hardcoded 1.49 in LID.cppconstants::PHI

All local usages now reference constants::PHI / constants::GRAVITY from the single header:

// Before (scattered across files)
static constexpr double PHI = 1.486;
g.surf_alpha[us] = 1.49 * std::sqrt(g.surf_slope[us]) / g.surf_rough[us];

// After (single source of truth in Constants.hpp)
constexpr double PHI = 1.4859;
// Usage sites reference the central constant
g.surf_alpha[us] = constants::PHI * std::sqrt(g.surf_slope[us]) / g.surf_rough[us];

Copilot AI and others added 4 commits May 28, 2026 07:11
…mum depth

When a weir (or other link) opening exceeds the rim of a storage node,
no warning was previously issued. For non-storage nodes, WARN02 fires
when node_validate detects the fullDepth was increased in link_validate.
Storage nodes skip the fullDepth extension (correctly, since storage
volume is defined by user-configured curves), so WARN02 never fired.

Add WARN13 "WARNING 13: link opening exceeds maximum depth for Node"
to text.h, and issue it in link_validate when a link crown height
exceeds the fullDepth of a storage node (with no surcharge depth).

Fixes: Maximum depth warning missing for weir opening exceeding storage
node rim.
…ning

Add WARNING 13 for link opening exceeding storage node maximum depth
Copilot AI linked an issue May 28, 2026 that may be closed by this pull request
…ient

- Update GRAVITY from 32.2 to 32.174 (US customary, ft/s²)
- Update SI_GRAVITY from 9.81 to 9.80665 (standard gravity, m/s²)
- Update PHI from 1.486 to 1.4859 (Manning equation unit factor)
- Replace hardcoded 1.49 with PHI in lid.c (3 occurrences)
- Replace hardcoded 1.49 with PHI in transect.c (1 occurrence)
- Remove redundant MCOEFF constant from subcatch.c, use PHI instead

Resolves constants precision issue where values were applied
inconsistently and at lower precision than necessary.
Copilot AI changed the title [WIP] Update constants for improved precision Fix constants precision and eliminate hardcoded Manning coefficient May 28, 2026
Copilot AI requested a review from cbuahin May 28, 2026 10:46
Copilot AI changed the title Fix constants precision and eliminate hardcoded Manning coefficient fix: update constants precision and centralize definitions May 28, 2026
@cbuahin cbuahin changed the base branch from main to develop May 29, 2026 01:43
@cbuahin cbuahin closed this May 29, 2026
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.

Constants precision nits

2 participants