Skip to content

Update cfe.c#139

Merged
ajkhattak merged 2 commits intoNOAA-OWP:masterfrom
fred-ogden:patch-3
Mar 3, 2025
Merged

Update cfe.c#139
ajkhattak merged 2 commits intoNOAA-OWP:masterfrom
fred-ogden:patch-3

Conversation

@fred-ogden
Copy link
Contributor

Fixed major bug in xinanjiang function. One minor bass balance fix. Refactored. Verified code against NWM Fortran Xinanjiang function. Changed "infiltation_excess_params_struct" to "infiltration_excess_params_structure". Added "#ifdef DEBUG #endif around mass balance warning print statement at end of xinanjiang scheme function.

[Short description explaining the high-level reason for the pull request]

Additions

  • None

Removals

  • none

Changes

  • Major bug fix: if ( ( W / Wmax ) <= (0.5 - a)) {
    // THIS LINE IN ORIGINAL CFE XINANJIANG CODE IS A BUG
    // f_over_F = (pow((0.5 - a),(1.0 - b)) * pow((1.0 - (tension_water_m/max_tension_water_m)) , b));

    // FLO correct comparing against Eqn. 2a in Jayawardena & Zhou (2000)
    f_over_F = (pow((0.5 - a),(1.0 - b)) * pow( W / Wmax , b)); // Eqn. 2a in Jayawardena & Zhue

    } else { // if ( (0.5-a) < W / Wmax )
    f_over_F = 1.0 - pow((0.5 + a), (1.0 - b)) * pow((1.0 - W / Wmax) , b); // Eqn. 2b in Jayawardena & Zhue
    }

Minor bass balance bug fix: //edited by RLM; added logic block to handle what happens when porosity or field capacity = 0
//FLO changed from 0.95 to 1.0 because what happens to the other 5% ?
if (max_free_water_m <= 0.0 || max_tension_water_m <=0.0) {
*infiltration_excess_m = 1.0 * water_input_pervious_fraction_m + impervious_runoff_m;
*infiltration_excess_m += impervious_runoff_m; // must add impervious runoff back in
*infiltration_depth_m = 0.0; //added by FLO
return; // this is an unusual situation and should only happen when one or both parameters are zero
}

Testing

Screenshots

Notes

  • Verified with Xinanjiang() Fortran routine in NWM3.0 within 1.0e-05.

Todos

  • None

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (link if applicable)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Visually tested in supported browsers and devices (see checklist below 👇)
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Target Environment support

  • Windows
  • Linux
  • Browser

Accessibility

  • Keyboard friendly
  • Screen reader friendly

Other

  • Is useable without CSS
  • Is useable without JS
  • Flexible from small to large screens
  • No linting errors or warnings
  • JavaScript tests are passing

fred-ogden and others added 2 commits March 3, 2025 11:19
Fixed major bug in xinanjiang function.  One minor bass balance fix.  Refactored.  Verified code against NWM Fortran Xinanjiang function.  Changed "infiltation_excess_params_struct" to "infiltration_excess_params_structure".   Added "#ifdef DEBUG #endif around mass balance warning print statement at end of xinanjiang scheme function.
Copy link
Contributor

@ajkhattak ajkhattak left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks @fred-ogden for finding and fixing the bug

@ajkhattak ajkhattak merged commit e041fbb into NOAA-OWP:master Mar 3, 2025
4 checks passed
donaldwj added a commit that referenced this pull request Jan 16, 2026
* update submodule url to gitlab

* Implementation of the new Logging mechanism for C-based modules - NGWPC-3889

* fix to CFE unit test for mass balance check

* More changes including the move of setup_logger call to BMI initialize() function in bmi_cfe.c

* Final change to make it work with environment variable NGEN_LOG_FILE_PATH

* Revert "Merge branch 'NGWPC-3889' into 'development'"

This reverts merge request !3

* recommit of logger changes for CFE module

* NGWPC-4428 - CFE logging changes

* Logger optimization

* Added release number, date and commit hash of official release

* Adding feature for dynamically setting log level systemwide

* changed the way logLevel char pointer is checked for NULL

* Revert "changed the way logLevel char pointer is checked for NULL"

This reverts commit a7e8451.

* Revert "Adding feature for dynamically setting log level systemwide"

This reverts commit 2945134.

* Updated version file for release rc-1.1.0

* enhancement to allow flexible order in cfe config

* Update version file for release rc-1.1.0A

* Updated license file to reflect OWP government language

Added Apache license and copyright information for cfe repo via modification of existing LICENSE file

* Update version file for Release 1.1.0

* fix cfe error

* Update version file for master hotfix 1.1.1

* Added RTX licensing verbiage

* Remove duplicated RTX licensing verbiage

* Update version file for release rc-1.2.0 on 2025-02-12

* Update version file for release 1.2.0 on 2025-02-25

* Update cfe.c (#139)

* Update cfe.c

Fixed major bug in xinanjiang function.  One minor bass balance fix.  Refactored.  Verified code against NWM Fortran Xinanjiang function.  Changed "infiltation_excess_params_struct" to "infiltration_excess_params_structure".   Added "#ifdef DEBUG #endif around mass balance warning print statement at end of xinanjiang scheme function.

* fix: adjust white space for better review, fix struct parameter

---------

Co-authored-by: hellkite500 <nfrazier@lynker.com>

* ci: use pull_request_target when collecting project info for PR's from forks

* ci: us pull_request_target the right way...

* ci: remove duplicate event, pull_request is a subset of pull_request_target

* fix: don't double count impervious_runoff in infiltration_excess calculation

Co-authored-by: Nels <nels.frazier@noaa.gov>

* Fix aet, replaces #142 (#143)

* chore: small formatting refactor

Co-authored-by: ajkhattak <ajkhattak@gmail.com>

* feat: add method to compute et_from_retention_depth

Co-authored-by: ajkhattak <ajkhattak@gmail.com>

* fix: take AET from surface retention depth

Co-authored-by: ajkhattak <ajkhattak@gmail.com>

* fix: try to prevent segfault when computing AET from retention depth

---------

Co-authored-by: ajkhattak <ajkhattak@gmail.com>

* Delete version.txt

* Refactor and add env var checking

* Change ENABLE to ENABLED for EWTS flag

* build: broken reference to bmi_cfe header file

* Update logger to be consistent with other module loggers

* Change log levels ERROR to WARNING and WARNING to SEVERE

* Flush stdout after print

* Remove placholder comments

* Fix merge errors

* Fix header file reference

* Created serialization methods for CFE state data

* Change malloc to calloc to ensure null pointers

* Fix log level from SEVERE to WARNING for a warning.

* fix flux larger than storage error

* changed warning msg per OWP request

* Express dimensionless units of ice_fraction_xinanjiang and soil_moisture_profile as "1" so UDUNITS2 can parse them

* Throttle groundwater flux warning message to a single log entry.

* Adapt serialization to revised protocol

* ice_fraction_schaake (dimensionless)

* SURF_RUNOFF_SCHEME unit from None to 1 (dimensionless)

---------

Co-authored-by: Mohammed Karim <mohammed.r.karim@rtx.com>
Co-authored-by: Carolyn Maynard <carolyn.maynard@rtx.com>
Co-authored-by: areg.amirkhanian <areg.amirkhanian@rtx.com>
Co-authored-by: yuqiong.liu <yuqiong.liu@ertcorp.com>
Co-authored-by: Zhengtao Cui <zhengtao.cui@rtx.com>
Co-authored-by: David Gillingham <david.gillingham@rtx.com>
Co-authored-by: Shanti Surapaneni <shanti.surapaneni@rtx.com>
Co-authored-by: peter.a.kronenberg <peter.a.kronenberg@rtx.com>
Co-authored-by: Brian-Cosgrove <131808837+Brian-Cosgrove@users.noreply.github.com>
Co-authored-by: Fred Ogden <56281521+fred-ogden@users.noreply.github.com>
Co-authored-by: hellkite500 <nfrazier@lynker.com>
Co-authored-by: Nels <nels.frazier@noaa.gov>
Co-authored-by: ajkhattak <ajkhattak@gmail.com>
Co-authored-by: Austin Raney <austin.raney@noaa.gov>
Co-authored-by: Ian Todd <itodd@dewberry.com>
Co-authored-by: Yuqiong Liu <yuqiong.liu@U-28HBMJQ9PK1NS.nextgenwaterprediction.com>
Co-authored-by: Phil Miller <philip.miller@noaa.gov>
Co-authored-by: Phil Miller - NOAA <pmiller@lynker.com>
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