NOAA machine pass of base_suite #155

[rgrumbine]

For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers

#155

PR checklist

• Short (1 sentence) summary of your PR:
  Configurations to pass all regression tests from base_suite on NOAA machines hera, phase2, phase3

• Developer(s):
  Robert Grumbine

• Suggest PR reviewers from list in the column to the right.
  Tony Craig
  Dave Bailey
  Note: no column visible to the right

• [] Please copy the PR test results link or provide a summary of testing completed below.
  Bitwise pass of all tests in base_suite on all three systems

• How much do the PR code changes differ from the unmodified code?

  • [x ] bit for bit

• Does this PR create or have dependencies on Icepack or any other models?

  • [ x] No

• Does this PR add any new test cases?

  • [x ] No

• Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/. A test build of the technical docs will be performed as part of the PR testing.)

  • [x ] No, does the documentation need to be updated at a later time?
    • [x ] No

• Please provide any additional information or relevant details below:

[apcraig]

Why are there changes in icepack showing up? Is that intended?

[phil-blain]

I don't think it is; this PR references Icepack at commit 51ea326f12578355d4d51b0cea29b45cc9890cd1, which is not found in the Icepack repository...
Fetching the pull request locally results in an error :

error: Server does not allow request for unadvertised object 51ea326f12578355d4d51b0cea29b45cc9890cd1

[apcraig]

It's also causing the readthedocs test to fail, unable to find the icepack hash.

[rgrumbine]

No changes are intended in Icepack. I'm just still not up to speed on proper submodule use. Sorry, Bob

…

On Mon, Oct 21, 2019 at 5:43 PM Tony Craig ***@***.***> wrote: It's also causing the readthedocs test to fail, unable to find the icepack hash. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#372?email_source=notifications&email_token=AFQ65HPWSXN2WD7J2CEVHC3QPXS4NA5CNFSM4JDA67GKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEB3FWCY#issuecomment-544627467>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFQ65HL2JESBF6KJZXW23X3QPXS4NANCNFSM4JDA67GA> .

-- Tel: 301-683-3747 Coupling and Dynamics Group

[phil-blain]

Bob,

you can try these commands to update your branch :

git checkout master
git submodule update --remote --force
git add icepack
git commit -m "Revert icepack to 1.1.2"
git push 

[phil-blain]

@apcraig it is kind of troubling to me that the Travis build did not fail... when I try locally the commands from the Travis build log on my machine it gives the same error as RTD...
I can view the offending Icepack commit online here : CICE-Consortium/Icepack@51ea326
It comes from the commit 62d4854#diff-fdcb0c9501802a455921dc8b34a19a95 from Bob's master branch (which this PR references)

I think what happens is that since this PR references icepack at a commit that is not present in the icepack repository, GitHub creates an internal ref of some kind (in the CICE-Consortium/Icepack repository!) to be able to display the diff.
Although a mirror of the Icepack repo still doesn't find the commit...

$ git clone --mirror https://github.com/CICE-Consortium/Icepack.git icepack-mirror
$ cd icepack-mirror
$ git show 51ea326f12578355d4d51b0cea29b45cc9890cd1
fatal: bad object 51ea326f12578355d4d51b0cea29b45cc9890cd1

So Travis queries GitHub in such a way that the "unadvertised" commit is found...

[apcraig]

@phil-blain I was just looking at that too. I don't know where it's getting the icepack version. If you click from bob's master on the icepack #51ea326, you get here,

https://github.com/cice-consortium/icepack/tree/51ea326f12578355d4d51b0cea29b45cc9890cd1

So it seems to be a tree, whatever that is. I'm still trying to understand. I still don't know how it exists in the cice-consortium repo.

[phil-blain]

Maybe GitHub uses Git namespaces to store these hidden refs. However I haven't found a way to query the server for listings these alternate namespaces.

[phil-blain]

I found this blog post form GitHub which explains that they use the alternate mechanism for forks, and store the commits from every fork into the "root" repo of a fork network. This explains why Bob's commit appear in the Icepack repo.
To test it I just created a branch in my fork of CICE and pushed it, and it can be seen in the CICE official repo :
e876483 (changes in this commit)
https://github.com/CICE-Consortium/CICE/tree/e87648356c7d800156ef2e4e1a5a3895c46c2390 (repo tree for this commit)

However it doesn't explain why Travis can fetch the unadvertised objects...

[apcraig]

I think we can merge this. I would propose a couple minor changes. In the cice.batch.csh file, there are a couple of reference to @rgrumbine's email or home directories in the batch submission part. The same is true in the env.hera_intel file and the phase2 and phase3 files where the WKDIR and some other settings are assocaited with @rgrumbine's user name. This will not allow the codes to work for other users on those machines. Those should be made generic. You can see how that is done for other machines that already exist. Use of $user or system settings like $WORKDIR should be used instead. The mailing in PBS is harder to make generic but the approach used on other machines is to make it generic ([email protected]) but then comment it out. Again, there are examples for other machines.

[eclare108213]

@rgrumbine
Please, can you remove the script references that are specific to you, as outlined by Tony above? There are generic ways of doing this, which should work in your cases too. Thanks -

[rgrumbine]

I've made the changes in my fork, branch phase3_6.0.2.0 $WORKDIR isn't defined on our systems, but will cause a prompt crash if not redefined by user. I've left the input file reference to my copies, rather than have everybody make their own. Particularly relevant with the JRA55 input data files. Vacation went well. Son thoroughly married now. Bob

…

On Mon, Oct 28, 2019 at 3:09 PM Elizabeth Hunke ***@***.***> wrote: @rgrumbine <https://github.com/rgrumbine> Please, can you remove the script references that are specific to you, as outlined by Tony above? There are generic ways of doing this, which should work in your cases too. Thanks - — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#372?email_source=notifications&email_token=AFQ65HIUHMQX2AQTR6YZORLQQ42E7A5CNFSM4JDA67GKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECOBTTI#issuecomment-547101133>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFQ65HLWZ7DZ7TPJNXENARTQQ42E7ANCNFSM4JDA67GA> .

-- Tel: 301-683-3747 Coupling and Dynamics Group

[apcraig]

@rgrumbine I don't think you pushed your changes to this branch, which happens to be your master. Also, you don't have to use $WORKDIR, you should just use whatever construct makes sense on your machine, like /scratch4/$user/$CASE or something. On most machines, you can provide a generic implementation.

Finally, this branch has conflicts with the Consortium version now that will need to be resolved, but should be trivial. I think they just have an overlapping change of the same thing . If you want any help with this, I'm happy to do that. I could pull your changes onto a branch in my fork and create a PR. Or I could just make specific suggestions about what to change. Also, we generally do not recommending doing development on the master branch in your fork, if you want some help reverting your master fork back to the consortium master, I can help you with that too.

[apcraig]

@rgrumbine, just pinging. Would like to get this merged if we can. We had some email contact about next changes. How is that going? If you need any assistance with the conflict or otherwise, let me know. There are a few ways I could help.

[apcraig]

I think we could merge this, but there are still places where "Grumbine" shows up, namely in the email address in the batch scripts. I recommend that those be fixed too unless @rgrumbine wants mails from other peoples jobs on those machines.

[eclare108213]

@rgrumbine
Are you happy with these code changes as they are? We would rather not have specific references to your email and file directories, but if that's they way you prefer these machines be handled, we will go with it.

[apcraig]

I am going to merge this and then independently test on hera. I may also try to port to Orion if I get a chance.