search

NOAA-OWP / ngen

Next Generation Water Modeling Engine and Framework Prototype
Other
84 stars 63 forks source link

Updated realization configs in `data/` #788

Open SnowHydrology opened 7 months ago

SnowHydrology commented 7 months ago

787 indicates that the example realization configs in the data directory are out of date. Notably, none of the realizations mapped Noah-OWP-Modular QINSUR output to CFE, meaning the latter was taking precipitation directly from forcing.

Additions

-

Removals

-

Changes

Updated the variables_names_map in:

Testing

1.

Screenshots

Notes

-

Todos

-

Checklist

Testing checklist (automated report can be put here)

1.

Target Environment support

SnowHydrology commented 7 months ago

I set this as draft because I have a couple questions:

stcui007 commented 7 months ago

The following list current cfe module input variables:

https://github.com/NOAA-OWP/cfe/blob/master/src/bmi_cfe.c#L278-L283

On Thu, Apr 4, 2024 at 7:33 AM K. Jennings @.***> wrote:

I set this as draft because I have a couple questions:

— Reply to this email directly, view it on GitHub https://github.com/NOAA-OWP/ngen/pull/788#issuecomment-2037067095, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACA4SRP7O4WAXJOVAXNIXZDY3VCBLAVCNFSM6AAAAABFXFRZGKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZXGA3DOMBZGU . You are receiving this because your review was requested.Message ID: @.***>

SnowHydrology commented 7 months ago

Thanks for that info, @stcui007. I could've been clearer in my question for @ajkhattak. CFE will set those values if running uncouple (example here), so I was hoping Ahmad could check if my assumptions were correct.

stcui007 commented 7 months ago

@ajkhattak https://github.com/ajkhattak is the right person to answer that question.

On Thu, Apr 4, 2024 at 8:45 AM K. Jennings @.***> wrote:

Thanks for that info, @stcui007 https://github.com/stcui007. I could've been clearer in my question for @ajkhattak https://github.com/ajkhattak. CFE will set those values if running uncouple (example here https://github.com/NOAA-OWP/cfe/blob/6d08db7bec2ec6cc2cc2446300c9e0d2076e0fc7/src/cfe.c#L125), so I was hoping Ahmad could check if my assumptions were correct.

— Reply to this email directly, view it on GitHub https://github.com/NOAA-OWP/ngen/pull/788#issuecomment-2037267885, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACA4SRPEPWSHDPFTMEFJTCDY3VKQJAVCNFSM6AAAAABFXFRZGKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZXGI3DOOBYGU . You are receiving this because you were mentioned.Message ID: @.***>

ajkhattak commented 7 months ago

@SnowHydrology

CFE does require SLoTh outputs if running in the framework in standalone mode (i.e., not coupled with SMP (soil moisture profiles) and SFT (soil freeze-thaw)) this looks to me like the updated version that has the correct SLoTH+CFE mapping, so we should be using this one.

this example is outdated.

Just a note: NONE of the soil models (other than the topmodel) will run in the framework without SLoTH outputs.

ajkhattak commented 7 months ago

Thanks for that info, @stcui007. I could've been clearer in my question for @ajkhattak. CFE will set those values if running uncouple (example here), so I was hoping Ahmad could check if my assumptions were correct.

@SnowHydrology One clarification here, CFE takes ice_fraction_xinanjiang as an input from SFT, however, if SFT is not coupled than it comes from SLoTH. So a user through SLoTH might, by mistake, set it to a non-zero number so these lines 

if(!soil_reservoir_struct->is_sft_coupled)
      soil_reservoir_struct->ice_fraction_xinanjiang = 0.0;

ensure that ice_fraction_xinanjiang is zero if SFT is not coupled otherwise it will lead to incorrect runoff calculations.

stcui007 commented 7 months ago

I have done tests running ngen with all realizations except example_bmi_multi_realization_config__macos.json and example_bmi_multi_realization_config_w_routing.json in both serial and parallel mode, as the automatic test has only limited coverage. They all run successfully on local Linux servers. Maybe Keith alreay done this. Then this just confirm it.

SnowHydrology commented 7 months ago

Thanks @stcui007! I think we're just waiting on a quick look from @hellkite500 to determine which configs are still used and which can be deleted.

stcui007 commented 4 months ago

Since this one has been there for a long time, I decided to take a look at it together with PR#775 to set them up to merge.

To @hellkite500's question about those test_... realization configs, I believe they are not used currently. They were probably meant to test the configurations with some explicit specification for some cat-id.

The example_bmi_multi_realization_config_w_routing.json might possibly be out of date and be removed as well.

In realizations involve both PET and CFE, depending on preference, the following line of mapping is not absolutely necessary, I think.

"water_potential_evaporation_flux": "potential_evapotranspiration"