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

fix to logic in restarting when using a prior case's restart #171

Merged
merged 1 commit into from
Jan 18, 2017
Merged

fix to logic in restarting when using a prior case's restart #171

merged 1 commit into from
Jan 18, 2017

Conversation

ckoven
Copy link
Contributor

@ckoven ckoven commented Jan 6, 2017

fix to properly restart when using finidat from a prior case

Fixes: [NGT-ED Github issue #165 ]

User interface changes?: [No]

Code review: [just me so far, @rgknox -- could you double check that this makes sense to you too?]

Test suite: ED test suite on lawrencium-lr3
Test baseline: don't have any recent baselines, so didn't run; I can't imagine this would be an issue but please double-check when testing on Yellowstone
Test namelist changes: none
Test answer changes: ought to be b4b

Test summary: all smoke, restart, etc tests pass

@rgknox
Copy link
Contributor

rgknox commented Jan 6, 2017

This change will indeed prevent the cold-start initialization sequence from happening if finitdat is not specified.
My questions (which I can look into as well): 1) is the reading of restart data happening when finidata is specified (these changes may only turn off coldstart)
2) I'm wary that there are going to be confusions between finidat = ' ' and finidat = ' '. Maybe we should add a function in there that strips white space off the finidat and compares it to a null string or something.

@bandre-ucar
Copy link
Contributor

I believe the finidat = ' ' with a space is intentional, at least as far as clm is concerned, but I don't remember the reason. This should be run by the clm-cmt.

@rgknox
Copy link
Contributor

rgknox commented Jan 6, 2017

After reviewing the code...
Following up on question 1: the reading of the fates restart data should be happening when finitdat is specified, and from reading the code I have no suspicions that it is not. See clm_initializeMod.F90:

is_cold_start = .false.

    if (nsrest == nsrStartup) then

       if (finidat == ' ') then
          if (finidat_interp_source == ' ') then
             is_cold_start = .true.
             if (masterproc) then
                write(iulog,*)'Using cold start initial conditions '
             end if
          else 
             if (masterproc) then
                write(iulog,*)'Interpolating initial conditions from ',trim(finidat_interp_source),&
                     ' and creating new initial conditions ', trim(finidat_interp_dest)
             end if
          end if
       else 
          if (masterproc) then
             write(iulog,*)'Reading initial conditions from ',trim(finidat)
          end if
          call getfil( finidat, fnamer, 0 )
          call restFile_read(bounds_proc, fnamer, glc_behavior)
       end if

    else if ((nsrest == nsrContinue) .or. (nsrest == nsrBranch)) then

       if (masterproc) then
          write(iulog,*)'Reading restart file ',trim(fnamer)
       end if
       call restFile_read(bounds_proc, fnamer, glc_behavior)

    end if

restFile_read would be called in such a case, which will ultimately lead to the calling of clm_fates%init_restart()

Following up on question 2: the string comparison of finidat == ' ' seems to be the status quo in that part of the code, so I suppose it is fine, (and I'm just now reading Ben's commend above)

@ckoven
Copy link
Contributor Author

ckoven commented Jan 8, 2017

closing as the change is also in #172.

@ckoven ckoven closed this Jan 8, 2017
@ckoven ckoven reopened this Jan 9, 2017
@ckoven ckoven assigned ckoven and bandre-ucar and unassigned ckoven Jan 9, 2017
@bandre-ucar
Copy link
Contributor

testing

@bandre-ucar
Copy link
Contributor

test status:

  • clm_short - ys - pass
  • ed - ys - pass (1 Lm6 test still running).
  • ed - hobart - mpi/network errors, rerunning.

@bandre-ucar bandre-ucar merged commit 8e8dece into NGEET:master Jan 18, 2017
@bandre-ucar bandre-ucar removed their assignment Apr 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants