NOAA-OWP / evapotranspiration

Other
3 stars 9 forks source link

Update BMI spec to remove precipitation #10

Closed SnowHydrology closed 2 years ago

SnowHydrology commented 2 years ago

Removals

Changes

Testing

  1. Ran PET code and unit tests

Todos

Checklist

Target Environment support

SnowHydrology commented 2 years ago

@madMatchstick and @lcunha0118, I can convert this to draft if you think we should modify PET further as noted in the "Todos" above

Todos

Should we modify structs to remove all potential to read in/store precipitation data? Would be more intensive than just modifying the BMI specification.

madMatchstick commented 2 years ago

@madMatchstick and @lcunha0118, I can convert this to draft if you think we should modify PET further as noted in the "Todos" above

Todos Should we modify structs to remove all potential to read in/store precipitation data? Would be more intensive than just modifying the BMI specification.

Don't think removing aorc.precip_kg_per_m2 completely is really necessary here? If this is what you're asking. (But perhaps what is would be (re)moving AORC to separate repo/submodule, another topic... :) ).

Although, it wouldn't hurt to remove any get/set_value() or print statements within current build/run scripts; e.g. here and here

SnowHydrology commented 2 years ago

Although, it wouldn't hurt to remove any get/set_value() or print statements within current build/run scripts; e.g. here and here

That is a good catch. I can remove those. Did you run a comprehensive search or those were just two that stood out?

SnowHydrology commented 2 years ago

@madMatchstick I removed precip from get/set and from the print statements in both main programs. Unit tests and both main programs display expected behavior. I think this should be good for now unless you catch anything else. Thanks!

madMatchstick commented 2 years ago

Although, it wouldn't hurt to remove any get/set_value() or print statements within current build/run scripts; e.g. here and here

That is a good catch. I can remove those. Did you run a comprehensive search or those were just two that stood out?

These just stood out. I did confirm there are no others (excluding aorc_bmi, that is).