Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ED2 versions in BETY models table based on git hash instead of generic "git" #2081

Closed
serbinsh opened this issue Aug 31, 2018 · 9 comments

Comments

@serbinsh
Copy link
Member

Is your feature request related to a problem? Please describe.
Presently we are using a generic "git" version in BETY for ED2 (including 3 separate entries for ED2 version git) which makes it 1) hard to keep track of which BETY record for git corresponds to which machine and 2) causes problems when the ED2IN file changes with git but not in PEcAn or changes in PEcAn and ED2 but a machine's version of ED2 is behind a version which will create a run error in PEcAn if the user selects the PEcAn "git" version of the file that ships with PEcAn.

Describe the solution you'd like
We would include the latest main ED2 branch git hash in the model table for the ED2 version, for example:

[sserbin@modex ED2.git]$ git log --pretty=format:'%h' -n 1
5065966

which is the hash for PR 250

commit 5065966be069b9cfdb3b5331d86d7c437c104677
Merge: aeaaa7d cf2e960
Author: Shawn P. Serbin <sserbin@bnl.gov>
Date:   Wed May 23 10:22:17 2018 -0400

    Merge branch 'master' of github.com:EDmodel/ED2 into EDR_update

commit cf2e9609fb10c41f238ba988276bcebe9da5c414
Merge: 2014a1f 86b6cf0
Author: Marcos Longo <mdplongo@gmail.com>
Date:   Sun Apr 29 08:25:40 2018 -0700

    Merge pull request #248 from xiangtaoxu/hydro_pr

    fix bug for wood energy budget when IBRANCH_THERMO = 0 under plant hydro

commit 86b6cf0c38599d4d9fc34826337859e4c0d0a86a
Author: Xiangtao Xu <xu.withoutwax@gmail.com>
Date:   Thu Apr 26 10:30:41 2018 -0400

    revise some parenthesis usages....

We would then also name the ED2IN files in inst, for example ED2IN.5065966 or something similar such that ED2 version and ED2IN files can be linked. OR perhaps this just needs to happen for releases where ED2IN changes, though I could see that also being challenging to keep track of.

What do folks think? I may not have done a good job with the explanation but I think there must be a better way of keep track of versions and ED2IN's for ED2.

@robkooper
Copy link
Member

One issue is that pecan will use the version to find the right config files for ED in the install folder, so for the git version it will look for a file called ED2IN.rgit. So if we don't see the appropriate file, we can fall back to that version

Secondly, keep in mind that if you have some local modifications of ED, everytime there is a git commit on the remote site, you might end up with a merge commit, resulting in a hash that is unique to only you site.

That being said, I do think this is the best option. In the case of docker I might have to figure out a new naming scheme. Right now the images are called pecan/model-<modeltype>-<modelversion>:<pecanversion>. So if we want to use the hash there as well we might end up with a lot of images (which is ok).

@robkooper
Copy link
Member

I think this applies to a lot of other models as well. Anything that is build that is not an official release should follow this scheme.

@serbinsh
Copy link
Member Author

@robkooper agreed....I was wondering if we could stick to main branch hashes not custom end-user tweaks to save in PEcAn....and any local configurations could use their own ED2IN.[hash] files? Though I realize this is not a great solution but we need a better way to track ED2 versions and ED2IN versions

@ashiklom
Copy link
Member

ashiklom commented Oct 5, 2018

The Docker Hub model of giving each image its own tag, and then having a "latest" tag that is tied to the latest stable version, is a good idea. Not sure how best to automatically configure that alias though.

@ashiklom
Copy link
Member

ashiklom commented Oct 5, 2018

In the meantime, I think we should just abandon the -git tag altogether and rigorously enforce model versions tied to git hashes and tags. Notably, we don't have to build every single git commit.

@robkooper
Copy link
Member

As for configuration file, we can use the same we use currently and maybe fall back on ED2IN with no version which we assume will work with any latest version. Best is to check in the ED2IN. file though.

@ashiklom
Copy link
Member

I should add to this thread that @serbinsh indicated that ED2IN files with tags unknown to the current version of ED will cause it to crash. In other words, using a newer ED2IN that has more stuff can break an older version of ED.

@github-actions
Copy link

This issue is stale because it has been open 365 days with no activity.

@robkooper
Copy link
Member

now using ED tag 2.2 (as well as git), should eventually phase out git out of the official builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants