Setting up environment for Evapotranspiration submodule in ngen

[stcui007]

Additions

extern/evapotranspiration/evapotranspiration

Removals

extern/evapotranspiration/CMakeLists.txt
extern/evapotranspiration/petbmi.pc.in

Changes

.gitsubmodules
README.md

Testing

1. Clone the submodule
2. Build the libpetbmi shared library

Screenshots

Notes

Todos

Checklist

• [ x] PR has an informative and human-readable title
• Changes are limited to a single goal (no scope creep)
• [x ] Code can be automatically merged (no conflicts)
• Code follows project standards (link if applicable)
• Passes all existing automated tests
• [x ] Any change in functionality is tested
• [x ] 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)
• [x ] Reviewers requested with the Reviewers tool ➡️

Testing checklist (automated report can be put here)

Target Environment support

• [x ] Linux

[mattw-nws]

So, I believe this needs to be revised in several areas. Foremost:

The commit that the submodule is pinned to is one prior to the merging of the related submodule PR...so when checking out this branch of ngen, the extern/evapotranspiration/evapotranspiration directory has no CMakeLists.txt file. It should point to 8464f56 or later.

Also...

I don't believe we need the additional directory level after the submodule PR... there is nothing there except the README.md now. Believe we should remove it... though that will require documentation updates on the submodule side. It may make sense to add a README.md to the extern directory directly to address the updating of submodule versions/commits generally for submodules directly at this level (t-route and UDUNITS-2 are already at this level, so directions should be written to apply equally to them). This documentation can be done later as it's missing for those other modules already, but we should add an issue for it.

[stcui007]

Thanks, Matt. Will implement the suggestions.

…

On Fri, Feb 25, 2022 at 1:01 PM Matt Williamson ***@***.***> wrote: So, I believe this needs to be revised in several areas. Foremost: The commit that the submodule is pinned to is one prior to the merging of the related submodule PR...so when checking out this branch of ngen, the extern/evapotranspiration/evapotranspiration directory has no CMakeLists.txt file. It should point to 8464f56 <NOAA-OWP/evapotranspiration@8464f56> or later. Also... I don't believe we need the additional directory level after the submodule PR... there is nothing there except the README.md now. Believe we should remove it... though that will require documentation updates on the submodule side. It may make sense to add a README.md to the extern directory directly to address the updating of submodule versions/commits generally for submodules directly at this level (t-route and UDUNITS-2 are already at this level, so directions should be written to apply equally to them). This documentation can be done later as it's missing for those other modules already, but we should add an issue for it. — Reply to this email directly, view it on GitHub <#383 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACA4SRM2UGZL2ZIWI7EADV3U47GWZANCNFSM5OESIKSQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[stcui007]

For the moment, the structure extern/evapotranspiration/evapotranspiration directory structure kept unchanged. One consideration is that the formulation team is currently using this directory structure in their realization config file for PET lib in their code testing. Perhaps this can be adjusted later as you suggested.

[hellkite500]

One consideration is that the formulation team is currently using this directory structure in their realization config file for PET lib in their code testing

WRT library locations, we can always use cmake -DCMAKE_INSTALL_PREFIX=/custom/ngen/lib/path when configuring the submodule builds and then simply make install and the library, regardless of source tree/submodule structure, will be available in /custom/ngen/lib/path

[mattw-nws]

@stcui007 This still needs to have the submodule commit updated to 8464f56 or later. Disregard, sorry...I think I just need to update.

[mattw-nws]

This looks good other than one mildly pedantic question... what is the name of this module? Are we calling it "PET", or "evapotranspiration"? Both? Docs say PET and pet (lowercase)... do we care? @madMatchstick @SnowHydrology ?

[SnowHydrology]

This looks good other than one mildly pedantic question... what is the name of this module? Are we calling it "PET", or "evapotranspiration"? Both? Docs say PET and pet (lowercase)... do we care?

The repo is evapotranspiration but technically the code produces PET, or potential evapotranspiration. I.e., calling it PET is good.

[mattw-nws]

Approved with slight documentation changes.

[mattw-nws]

hmm... I thought I cleared the merge conflicts but apparently not. Inferring this must be conflicting because of the submodule commits in #343.

[SnowHydrology]

@mattw-nws, this PR points to the wrong version of PET. Please update to https://github.com/NOAA-OWP/evapotranspiration/tree/1e971ffe784ade3c7ab322ccce6f49f48e942e3d