no monthy or daily if ref_time is not '000000'

[weiyuan-jiang]

No description provided.

[tclune]

Do you really want a new library for this? The add_executable() macro allows you to specify multiple sources.

[gmao-rreichle]

@tclune : The actual (2-dot) changes here are minimal. Not sure why you're getting messages. Perhaps something went wrong with Weiyuan's merge of develop into this branch.
There's no touching of CMake or anything else of note. We're just adding a check in the job file (lenkf.j) that verifies consistency of two HISTORY parameters.

[tclune]

Matt and I review all changes to the CMake files. This allows us to ensure consistency for the build system over time.

And there were changes to the CMakeLists.txt for the APP in this PR.

[weiyuan-jiang]

The CMakeLists.txt changes are already in develop branch. This branch simply bring develop into it. So I believe the check is based on three dot comparison not two dot comparison.

[tclune]

GitHub clearly shows changes to CMake going into develop. Regardless, my comment is about whether you really want/need an extra library in the App directory. I'm going to approve now in the interest of speed. But please think about this.

[weiyuan-jiang]

Yes, at this moment, we need an extra library in the App directory. But in the future, we may move all those preprocessing subroutines to a new location.

[gmao-rreichle]

@tclune , @weiyuan-jiang , @mathomp4 :

At the risk of taking out of proportion... I'm still trying to understand what Tom's concern is here:

Do you really want a new library for this? The add_executable() macro allows you to specify multiple sources.

I'm guessing this refers to the following change:

This was approved previously by Tom and Matt in PR #283.

Would it be better to have preprocess_ldas_routines.F90 live in a different directory?
If so, where should it live?

It should be easy to make further changes. Note that we're about to make a full release of GEOSldas (after 8 beta releases), so now's a good time to make such a change.

[tclune]

Yes - that is the code in question, but the concern is different. And yes, I could have caught this change in the earlier PR, but sometimes I focus a bit more than others.

There is simply no need for the esma_add_library() command. The add_executable() command could simply absorb the new sourcefile. Delete lines 1-7 and change lines 20-24 to be:

   ecbuild_add_executable (	
      TARGET ${prog}.x 
      SOURCES ${prog}.F90	preprocess_ldas_routines.F90
      LIBS GEOSldas_GridComp mk_restarts)

We usually introduce libraries either because the source is needed in multiple executables or more commonly because the source is used by other layers of the model.

[tclune]

Ah - and it did not help that the GitHub diffs summary was hiding he loop over multiple executables. So there is some rationale for the library after all. One could still argue my change is simpler, but ... down in the weeds at this point.

[gmao-rreichle]

Addresses item 1.) of issue #204
Looks good to me. I approve assuming that it's been 0-diff tested.