-
Notifications
You must be signed in to change notification settings - Fork 349
fix(mfusg): fix precision loss in CLN/GNC packages writing coordinates #2658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
fix(mfusg): fix precision loss in CLN/GNC packages writing coordinates #2658
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2658 +/- ##
===========================================
+ Coverage 55.5% 72.6% +17.1%
===========================================
Files 644 667 +23
Lines 124135 129360 +5225
===========================================
+ Hits 68947 93990 +25043
+ Misses 55188 35370 -19818
🚀 New features to boost your workflow:
|
|
@reneangermeyer the USG executable seems not to like something about the new format? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a critical precision loss bug in MFUSG's CLN and GNC packages by updating the float format specifier from %10.2e (only 3 significant figures) to %16.9G (9 significant figures). This precision loss was causing measurable differences in model results.
- Changed the format string for float values in the shared
fmt_string()function - The fix applies to both CLN and GNC packages which import this function
- Increases precision from 3 to 9 significant figures, exceeding float32's 7-digit precision
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MFUSG CLN/GNC packages were losing significant precision when writing node coordinates, causing coordinate errors up to 3.5 km and affecting model results. ## Root Cause The fmt_string() method in mfusgcln.py and mfusggnc.py used %10.2e format (only 3 significant figures) instead of the standard Util2d format %15.6G (9 significant figures). ## Impact on Real Data Complete line from CLN package showing precision loss: Before (bug - %10.2e): 1 1 0 5.75e+01 1.15e+03 0.00e+00 0 0 4.01e+05 6.90e+06 1.21e+03 4.01e+05 6.90e+06 1.15e+03 After (fixed - %16.9G): 1 1 0 57.4650002 1150.19897 0 0 0 400569 6903567 1207.66394 400569 6903567 1150.19897 The precision loss caused measurable differences in model results. ## Solution Changed fmt_string() to return "%16.9G" format, which: - Provides 9 significant figures (exceeds float32's 7-digit precision) - Uses adaptive format (decimal when readable, scientific when needed) - Maintains proper spacing with 16-character width - Consistent with other MFUSG packages (WEL/DRN/RIV use %G) ## Testing Verified with Valle Copiapo model (142 CLN nodes, 181 stress periods). Model results now match original with negligible differences.
2c41371 to
80c10a2
Compare
The previous fix used %16.9G unconditionally, which violates MODFLOW-USG's 10-character fixed-width field requirement for models without FREE format in the BAS file. This fix checks if FREE format is enabled: - With FREE: use %16.9G for full precision - Without FREE: use original %10.2e for compatibility Users who need to preserve coordinate precision (e.g., 57.465 vs 5.75e+01) should enable FREE format in their model. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
@wpbonelli The issue was that the test model doesn't use FREE format, so %16.9G violates the 10-character field width limit required by MODFLOW-USG. However, the precision loss with %10.2e is still a real problem (values like 57.465 become 5.75e+01) which can affect model results. The fix now checks if FREE format is enabled in the BAS file: if so, it uses %16.9G for full precision, otherwise, it falls back to the original %10.2e to maintain compatibility with fixed-width input. |
|
Ah, got it. Seems reasonable. |
MFUSG CLN/GNC packages were losing significant precision when writing and affecting model results.
Root Cause
The fmt_string() method in mfusgcln.py and mfusggnc.py used %10.2e format (only 3 significant figures) instead of the standard Util2d format %15.6G (9 significant figures).
Impact on Real Data
Complete line from CLN package showing precision loss:
Before (bug - %10.2e):
1 1 0 5.75e+01 1.15e+03 0.00e+00 0 0 4.01e+05 6.90e+06 1.21e+03 4.01e+05 6.90e+06 1.15e+03
After (fixed - %16.9G):
1 1 0 57.4650002 1150.19897 0 0 0 400569 6903567 1207.66394 400569 6903567 1150.19897
The precision loss caused measurable differences in model results.
Solution
Changed fmt_string() to return "%16.9G" format, which: