Skip to content

fix: validate values that could cause divide by 0#54

Merged
hellkite500 merged 4 commits intoNOAA-OWP:masterfrom
hellkite500:fix-div-by-zero
Feb 20, 2026
Merged

fix: validate values that could cause divide by 0#54
hellkite500 merged 4 commits intoNOAA-OWP:masterfrom
hellkite500:fix-div-by-zero

Conversation

@hellkite500
Copy link
Copy Markdown
Contributor

Closes #52 by validating several config and/or forcing values which could lead to divide by zero situations.

Changes

  • Prints to stderr and exists in most cases, but a default small wind_speed is used with a warning printed instead of a hard stop.

  • 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

  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)

  • Reviewers requested with the Reviewers tool ➡️

Copy link
Copy Markdown
Member

@aaraney aaraney left a comment

Choose a reason for hiding this comment

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

One comment to look at but i'll go ahead and approve this. Thanks, @hellkite500!

src/pet.c Outdated
Comment on lines +41 to +47
if (model->pet_params.wind_speed_measurement_height_m <= model->pet_params.zero_plane_displacement_height_m){
// Must be <=, if == then log(1)=0 and divide by 0 occurs
fprintf(stderr, "ERROR: wind_speed_measurement_height_m must be > zero_plane_displacement_height_m. Current values: %lf and %lf\n",
model->pet_params.wind_speed_measurement_height_m,
model->pet_params.zero_plane_displacement_height_m);
exit(EXIT_FAILURE);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this needs to be removed. If we look at

zm=wind_speed_measurement_height_m;   // is set to 2 m
zh=humidity_measurement_height_m;     // is set to 2 m
d= zero_plane_displacement_height_m;   // = d = 2/3 * veg_height

 Technically, d can be greater than 2.0 for most of the case, if that happens, it is reset based on zh (below)
 
  if (d >= zh) {
    d = 2.0/3.0 * zh;
  }
  
so (zm-d) will always be positive, and so is (zh-d) in the log terms of Ra

let's talk about it if you believe I am missing something

@hellkite500
Copy link
Copy Markdown
Contributor Author

I have relaxed the strict displacement and measurement height checks, and instead implemented the assumption that when displacement height > measurement height then the measured wind speed is just used as the 2M wind speed, since in this case the log profile isn't valid.

This check also would have caused a failure that was indirectly accounted for in at least one method where humidity measurement height was used to compute a new displacement and transform, so the aerodynamic resistance computation now ensures valid numerical conditions, and the aorc forcing model param is validated to have a humidity measurement height of 2 meters.

@aaraney aaraney requested a review from ajkhattak February 20, 2026 20:40
@hellkite500 hellkite500 merged commit 6c410f2 into NOAA-OWP:master Feb 20, 2026
2 of 4 checks passed
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.

un-checked divide by 0

3 participants