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

Bugfixes/jstassi/#185/fvsetup #186

Merged
merged 2 commits into from
Jul 25, 2022
Merged

Conversation

gmao-jstassi
Copy link
Contributor

  • setenv ARCHIVE in the fstats_run script, so that the script can be run in an environment where ARCHIVE is not defined correctly for the experiment
  • change "tail +2" syntax to "tail -n +2"

… to correct tail +2 syntax error (changed to tail -n +2).
…up script, so that the *.input files can be run using the runjob script. The codeID values were also updated, but these are not currently being used for anything.
@gmao-jstassi gmao-jstassi requested a review from a team as a code owner June 3, 2022 19:03
@gmao-jstassi gmao-jstassi added 0 diff The changes in this pull request have verified to be zero-diff with the target branch. bug fix labels Jun 3, 2022
Copy link
Collaborator

@rtodling rtodling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Joe,
I truly think the approach of updating the codeID by hand is a doomed approach. If we are going to have to do this for every change we make it will be a nightmare. We have to find a way for this information to be picked up the fly.
I don't mind approving your changes - the real fix in fvsetup is relevant; the others ones ... not sure - as I say, I don't mind taking them, but that information will fall behind again in no time.

@gmao-jstassi
Copy link
Contributor Author

Ricardo, the codeID and fvsetupID values are updated with the update_input_IDs.pl script, so it is not completely by hand. The codeID value is not used for anything, so the only time these values need to be changed is when fvsetup is updated. I agree that it would be nice if this could be automated. I will look into it.

@rtodling
Copy link
Collaborator

rtodling commented Jul 21, 2022

Hi Joe,

I see that you are wiring in the git ref git (tag) in the input files ... this is a bit of a nightmare. We need to find a way to centralize this somehow and not have to change each and every one of these files just because we made a new commit to GEOSadas fixture that has nothing to do w/ the input files. Can you think of something? I will take these as they are here for now, but as soon as I put another change to develop this will fall out of sync.

Sorry - I see you had answered a similar comment I made on this before ... still, my comment is valid. We need a solution for this.

@mathomp4
Copy link
Member

@rtodling I can say it is possible to fill these in at CMake time. With the right code, CMake can figure out what the git commit of its own source code is and then fill that in. For example, if I add to the GEOSgcm CMakeLists.txt:

find_package(GitInfo)
GIT_WC_INFO(${CMAKE_CURRENT_SOURCE_DIR} GEOSgcm)

this will fill in a bunch of variables as defined in the FindGitInfo.cmake macro we carry in ESMA_cmake. If I then print them out with:

include(CMakePrintHelpers)
cmake_print_variables(GEOSgcm_WC_REVISION_HASH)
cmake_print_variables(GEOSgcm_WC_REVISION)
cmake_print_variables(GEOSgcm_WC_REVISION_NAME)
cmake_print_variables(GEOSgcm_WC_URL)
cmake_print_variables(GEOSgcm_WC_ROOT)
cmake_print_variables(GEOSgcm_WC_LAST_CHANGED_DATE)
cmake_print_variables(GEOSgcm_WC_LATEST_TAG)
cmake_print_variables(GEOSgcm_WC_LATEST_TAG_LONG)

then at CMake time we see:

-- GEOSgcm_WC_REVISION_HASH="9e87124"
-- GEOSgcm_WC_REVISION="9e87124"
-- GEOSgcm_WC_REVISION_NAME="9e87124 main"
-- GEOSgcm_WC_URL="[email protected]:GEOS-ESM/GEOSgcm.git"
-- GEOSgcm_WC_ROOT="[email protected]:GEOS-ESM/GEOSgcm.git"
-- GEOSgcm_WC_LAST_CHANGED_DATE=""2022-07-20 15:58:27 -0400""
-- GEOSgcm_WC_LATEST_TAG="v10.22.3"
-- GEOSgcm_WC_LATEST_TAG_LONG="v10.22.3-13-g9e87124"

You could use this in combination with configure_file to insert into scripts. I did try something like this in the testsuites CMake when I converted from CVS to Git, though I might have gotten some things wrong.

@gmao-jstassi
Copy link
Contributor Author

gmao-jstassi commented Jul 22, 2022 via email

@rtodling rtodling merged commit 5cf2e88 into develop Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff The changes in this pull request have verified to be zero-diff with the target branch. bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants