Generated new surface data including soil order for Phosphorus Cycle.

[acme-y9s]

Modified the surface data tool source code files and clm namelist files to add the soil order into the surface data file, which is needed for scientific development of P Cycle.

CC

[bishtgautam]

• I would suggest reverting the changes made in the models/lnd/clm/tools/clm4_5/mksurfdata_map/src/Makefile.common.
• By the change of CSMDATA in models/lnd/clm/tools/clm4_5/mksurfdata_map/mksurfdata.pl, we are implicitly assuming that Titan would be the default machine on which ACME datasets would be generated. Are folks (@rljacob, @mt5555, others) ok with this change and should we document this somewhere on confluence?

[thorntonpe]

Since there are small differences in the files depending on machine, I think it makes sense to designate a single machine to make all the files, using the ACME code.

From: Gautam Bisht [mailto:notifications@github.com]
Sent: Thursday, July 09, 2015 6:09 PM
To: ACME-Climate/ACME
Subject: Re: [ACME] Generated new surface data including soil order for Phosphorus Cycle. (#268)

· I would suggest reverting the changes made in the models/lnd/clm/tools/clm4_5/mksurfdata_map/src/Makefile.common.
· By the change of CSMDATA in models/lnd/clm/tools/clm4_5/mksurfdata_map/mksurfdata.pl, we are implicitly assuming that Titan would be the default machine on which ACME datasets would be generated. Are folks (@rljacobhttps://github.com/rljacob, @mt5555https://github.com/mt5555, others) ok with this change and should we document this somewhere on confluence?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/268#issuecomment-120162286.

[bbye]

How does that work for folks who need to modify the surface datasets but don't have access to Titan?

[bishtgautam]

@bbye : I would recommend applying for an account on Titan. On the Computational Resources page, you can search for OLCF Account Request Form. Though, presently the LCF project id is listed as TBD for 2015/07-2016/06, but I believe it is cli112 (same as last year).

[bishtgautam]

@daliwang : The changes in Makefile.common have fixed a value of USER_FC, thereby not allowing the user to override the Fortran compiler. Are you sure we want that?

[acme-y9s]

@bishtgautam : I made the changes several months ago, but I still can remember If we do not give a value for USER_FC, we will fail to compile the code and generate the executable file on Titan. So, at the beginning of this file I mentioned that the changes just made for Titan. The user still can change the value USER_FC for other machine. I did not quit understand what you really mean ? We should not specify the value for USER_FC?

[bishtgautam]

@acme-y9s : USER_FC ------ Allow user to override the default Fortran compiler specified in Makefile. But, presently USER_FC is hard-coded to be pgf90 for all machines (not just for Titan), thereby defeating the purpose of it.

You have correctly pointed out in your comment at the beginning of the Makefile.common

# set up for titan
#setenv LIB_NETCDF /opt/cray/netcdf/4.3.2/PGI/141/lib
#setenv INC_NETCDF /opt/cray/netcdf/4.3.2/PGI/141/include
#setenv USER_FC pgf90
# end for titan

A user needs to set appropriate value for USER_FC, LIB_NETCDF and INC_NETCDF as environment variables for the script to work correctly and in the case of Titan, the above mentioned values are correct. But, there is no need for harding-coding those values within the Makefile.common.

[daliwang]

I got a problem to merge local "acme-y9s/surfdata_map" into my local "next" (after git reset --hard origin/next), error message below, any suggestion?

Auto-merging models/atm/cam/src/physics/cam/phys_control.F90
CONFLICT (content): Merge conflict in models/atm/cam/src/physics/cam/phys_control.F90
Automatic merge failed; fix conflicts and then commit the result.

[jgfouca]

Dali, this is fine, it just means that your branch and some branch that was merged into next touched the same lines of code. This will happen from time to time. You need to open the conflicting file and look for the following:

>>>>
*your_changes*
========
*their_changes*
<<<<

You need to look at the difference between yours and theirs and hopefully it will be obvious what the resolution would be. Once you're satisfied, run "git add models/atm/cam/src/physics/cam/phys_control.F90" then "git commit".

[mt5555]

And just to repeat Jim's point in a different way: Please dont simply through away "their changes" and put in "your changes". You probably will have to take a merger of code from both sections.

[bishtgautam]

models/atm/cam/src/physics/cam/phys_control.F90 is different in master and next.

145c145
<       liqcf_fix, regen_fix, demott_ice_nuc, &                                               
---
>       liqcf_fix, regen_fix, demott_ice_nuc, &
323c323
<                         do_clubb_sgs_out, do_tms_out, state_debug_checks_out, fix_g1_err_ndrop_out,     & !BSINGH - bugfix for ndrop.F90
---
>                         do_clubb_sgs_out, do_tms_out, state_debug_checks_out, fix_g1_err_ndrop_out,     &
406c406
<    if ( present(fix_g1_err_ndrop_out    ) ) fix_g1_err_ndrop_out     = fix_g1_err_ndrop 
---
>    if ( present(fix_g1_err_ndrop_out    ) ) fix_g1_err_ndrop_out     = fix_g1_err_ndrop
411c411
<    if ( present(convproc_method_activate_out))convproc_method_activate_out= convproc_method_activate 
---
>    if ( present(convproc_method_activate_out))convproc_method_activate_out= convproc_method_activate

Everyone starting a new branch from current head of master will have a similar conflict when they try to merge in next. Thus, phys_control.F90 in next should be updated.

[mt5555]

I think we need a merge of "master" into "next"

[bishtgautam]

I think we can create a new branch (dev-name/cam/phys-control-conflict-fix) starting from master. Merge dev-name/cam/phys-control-conflict-fixinto next. Finally, merge dev-name/cam/phys-control-conflict-fix into master.

At the end, one could rebase acme-y9s/surfdata_map branch from master and there should be no conflicts when merging this PR into next.

[jgfouca]

That's odd, most of those lines look identical on both branches. Is there a whitespace change? In any event, very easy conflicts to resolve.

[bishtgautam]

On second thought, Mark's suggestion of merging master into next is much simpler.

[daliwang]

I followed @jgfouca comments and have added models/atm/cam/src/physics/cam/phys_control.F90 into the merge. Since the acme-y9s/surfdata_map branch was originally branched from master, so we should solve the file difference (phys_control.F90) between master and next.

[daliwang]

This branch has been merged into next. The missing new input parameter file is now in ACME data svn repo. We kept the makefile.common unchanged and added a new makefile example Makefile.titan.

[daliwang]

@jgfouca and @jedbrown New question:

I repeated the merge tonight.

1. git branch acme-y9s/surfdata_map --track origin/acme-y9s/surfdata_map
2. git checkout next
3. git reset --hard origin/next
4. git merge --no-ff acme-y9s/surfdata_map

GIT merge did not complain file conflict anymore, but showed "Already up-to-date". However

git diff acme-y9s/surfdata_map:models/atm/cam/src/physics/cam/phys_control.F90 next: models/atm/cam/src/physics/cam/phys_control.F90

still shows the differences between two files. Any guidance? Thanks.

[bishtgautam]

I believe you need to 'revert the revert'. See the last section on https://git-scm.com/blog/2010/03/02/undoing-merges.html

[bishtgautam]

argh ... 'reverting the revert' will end by in a models/atm/cam/src/physics/cam/phys_control.F90 that can't be compiled.

[daliwang]

I just want to push the acme-y9s in a right way. Can I just push my next into origin/next this time?

[douglasjacobsen]

@daliwang No, you can't do that. Tomorrow morning when I get time I'll lay out some instructions on how to fix this.

[daliwang]

@jgfouca Thanks for this. I will wait and follow your instructions tomorrow.

Also for your information, I have also tried

1. git branch acme-y9s/surfdata_map-resolved --track origin/acme-y9s/surfdata_map
2. git checkout next
3. git reset --hard origin/next
4. git merge --no-ff acme-y9s/surfdata_map-resolved

I found out the acme-y9s/surfdata_map-resolved:models/atm/cam/src/physics/cam/phys_control.F90 is same with the one in origin/next now, but is different than master:models/atm/cam/src/physics/cam/phys_control.F90.

[douglasjacobsen]

@daliwang I'm going to add instructions to the "Help: Testing" confluence page, just so more people will likely see it in the future.

[daliwang]

@jgfouca Thanks. There is another PR related to Phosphorus Cycle which will involve many file conflicts. I will work with you careful on this.

[jgfouca]

@bishtgautam , reverting the revert is the right way. The commit can then be amended to fix the broken code.