CICE-Consortium / Icepack

Development repository for sea-ice column physics
Other
25 stars 133 forks source link

Reorder calls to neutral_Coefficients and frzmlt_bottom_lateral - bugfix #380

Closed TillRasmussen closed 2 years ago

TillRasmussen commented 2 years ago

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

PR checklist

phil-blain commented 2 years ago

Hi @TillRasmussen, the title of the PR becomes the title of the commit message when the PR is merged. Would it be possible to use a more explicit title that describes what the PR is doing in 50-70 characters ? thanks.

TillRasmussen commented 2 years ago

Hi @TillRasmussen, the title of the PR becomes the title of the commit message when the PR is merged. Would it be possible to use a more explicit title that describes what the PR is doing in 50-70 characters ? thanks Done. I did notice it by my time was allocated elsewhere all day.

apcraig commented 2 years ago

Curious why the prior commit, #378 is showing up in this PR.

It looks like only the "alt04" tests should be affected. We'll want to check that once we merge and run the full suite.

phil-blain commented 2 years ago

Curious why the prior commit, #378 is showing up in this PR.

It's not the previous commit on main, but rather a broken copy of that commit (probably created by a cancelled merge from main). If you checkout this pull request locally, and check the commit graph, we see this:

$ git fetch https://github.com/cice-consortium/icepack.git pull/380/head:pr/380
$ git log --oneline --decorate --graph -10 pr/380 upstream/main 
* df1d3ed Added method to check namelist errors (#379) daveh150 (24 minutes ago)  (upstream/main)
* b7148ee Fix two output conversion errors  (#378) Philippe Blain (Thu Dec 2 14:43) 
| * 5d19492 change order of calls to neutral_drag and frzmlt. Fix bug TillRasmussen (Tue 08:24 +0000)  (HEAD -> pr/380, upstream/pr/380)
| * d48640a remove old url Till Rasmussen (Oct 23 2020) 
| * 54125aa Fix two output conversion errors  (#378) Philippe Blain (Thu Dec 2 14:43) 
|/  
* 152bd70 protect merge of meltsliq when tr_snow is off (#376) Tony Craig (Fri Nov 12 10:41) 

And the diff for commit 54125aa contains a non-removed conflict marker, which is later removed in d48640a.

I think it would be better to rebase the branch on top of an updated mainand keep only the tip commit 5d19492.