Expand --version option

[nikosavola]

Description of changes

• palace wrapper script is expanded to have -v and --version options, this was missing before even though the actual executable had --version from Added --version command line flag for version information #395
• Version output from executable includes build dependency versions for MFEM, LibCEED, SLEPc, PETSc, ARPACK, nlohmann/json, fmt, scn and Eigen.

Example output

~/dev/palace/build/bin 𝝺 ./palace --version
>> /usr/bin/mpirun -n 1 /home/niko/dev/palace/build/bin/palace-x86_64.bin --version

Palace version: 0.14.0
Git commit: v0.14.0-49-g854e58cf-dirty

Build dependencies:
  MFEM: 4.8.1
  libCEED: 0.12.0
  SLEPc: 3.23.99
  PETSc: 3.23.99
  nlohmann/json: 3.11.3
  fmt: 10.2.1
  scn: 4.0.1
  Eigen: 3.4.0

The solution was generated with the assistance of the GitHub Copilot agent. The commit authors reflect this.

[hughcars]

Hi @nikosavola,

Thank you for the pr. I think the addition of -v to the wrapper script is very useful and a good addition. I am more lukewarm on the dependency information implementation here, because of Palace's idiosyncratic version updating scheme. The reported information with the dependency version gives an incomplete (and thus misleading) view of the dependency state especially in the case of a cmake build. In the case of a spack build, any information additionally ends up being redundant.

If the versioning information was instead built from the git shas within ExternalGitTags, if those dependencies are built, that would be probably more helpful and impossible to mislead. This would also play nicely with spack, which would then only report those dependencies we are building outside of the spack build system (a.k.a mfem). So it would be some variation on the section in palace/CMakeLists.txt

include(GetGitDescription)
git_describe(GIT_COMMIT_ID)
message(STATUS "Git string: ${GIT_COMMIT_ID}")
if(NOT GIT_COMMIT_ID MATCHES "NOTFOUND")
  set_property(
    SOURCE ${CMAKE_SOURCE_DIR}/main.cpp
    APPEND PROPERTY COMPILE_DEFINITIONS "PALACE_GIT_COMMIT;PALACE_GIT_COMMIT_ID=\"${GIT_COMMIT_ID}\""
  )
endif()

for each dependency as it's included with ExternalXXX.cmake. This would mean moving that helper function out to the superbuild scope and wrapping it in a function that will call from the appropriate dependencies scope.

This process is going to get a fair bit involved however, so its up to you to decide if you're prepared, it would be a cool feature though. It would be somewhat helpful in rapid diagnosing a dependency's state, but that information is ultimately encoded already, so it's a nice to have rather than presenting great utility.

If not, I suggest to just roll back to allowing the -v command on the wrapper script, and we can merge this.

[hughcars]

A first difficulty with this approach, is that the version numbers don't actually reflect all the information needed to reconstruct a build. In particular, the specific git sha in ExternalGitTags are often far in advance of the version numbers. This would suggest reporting the git sha instead. I.e. the mfem version has been 4.8 for months, but we've updated many times in between.

The information required to reconstruct the SHAs is all present in the palace git tag, whilst the information provided by these dependency versions is incomplete. If you wanted to know the dependency versions in the cmake build, you'd still have to go and look at the git sha in Palace.

A second difficulty then becomes the discrepancy between spack builds and cmake builds. In a cmake build this information is in theory useful, in a spack build it is somewhat useful (but redundant) but also more difficult to extract.

[hughcars]

This shouldn't be needed as the palace version information is already encoded via the existing git_describe(GIT_COMMIT_ID) call which will take the most recent tag and add commits since then. This is more accurate in terms of giving the version information, and the delta since. This is very helpful with builds of main which is where this type of version information is most helpful).

[nikosavola]

Considered in ecec3d9

[Sbozzolo]

I think the omission of -v as a short flag in the executable was intentional, to avoid confusion with verbose, see #394.

[nikosavola]

I think the omission of -v as a short flag in the executable was intentional, to avoid confusion with verbose, see #394.

I see, how about -V instead. This seems more standard. For example python obeys -V for version and -v for verbosity. I made the change in 8c9fe1f.

[nikosavola]

Hi @nikosavola,

Thank you for the pr. I think the addition of -v to the wrapper script is very useful and a good addition. I am more lukewarm on the dependency information implementation here, because of Palace's idiosyncratic version updating scheme. The reported information with the dependency version gives an incomplete (and thus misleading) view of the dependency state especially in the case of a cmake build. In the case of a spack build, any information additionally ends up being redundant.

If the versioning information was instead built from the git shas within ExternalGitTags, if those dependencies are built, that would be probably more helpful and impossible to mislead. This would also play nicely with spack, which would then only report those dependencies we are building outside of the spack build system (a.k.a mfem). So it would be some variation on the section in palace/CMakeLists.txt

include(GetGitDescription)
git_describe(GIT_COMMIT_ID)
message(STATUS "Git string: ${GIT_COMMIT_ID}")
if(NOT GIT_COMMIT_ID MATCHES "NOTFOUND")
  set_property(
    SOURCE ${CMAKE_SOURCE_DIR}/main.cpp
    APPEND PROPERTY COMPILE_DEFINITIONS "PALACE_GIT_COMMIT;PALACE_GIT_COMMIT_ID=\"${GIT_COMMIT_ID}\""
  )
endif()

for each dependency as it's included with ExternalXXX.cmake. This would mean moving that helper function out to the superbuild scope and wrapping it in a function that will call from the appropriate dependencies scope.

This process is going to get a fair bit involved however, so its up to you to decide if you're prepared, it would be a cool feature though. It would be somewhat helpful in rapid diagnosing a dependency's state, but that information is ultimately encoded already, so it's a nice to have rather than presenting great utility.

If not, I suggest to just roll back to allowing the -v command on the wrapper script, and we can merge this.

I experimented in f2e5b9b and now fetch the git describe results instead of using the project version from CMake. This is carried out in the build/extern/… folders and outputs something for most dependencies. However, this doesn't work for something like Eigen which appears to download from https://gitlab.com/libeigen/eigen/-/archive/3.4.0/eigen-3.4.0.tar.gz instead of something with a .git folder.

I don't know how this works with Spack. Let me know what you think.

[hughcars]

Thanks for working on this @nikosavola, it's looking really close! I tried this just now, and get

Git commit: v0.14.0-55-gef65c5fc

Build dependencies:
  STRUMPACK: GIT-NOTFOUND
  arpack_ng: GIT-NOTFOUND
  eigen: GIT-NOTFOUND
  fmt: GIT-NOTFOUND
  gslib: v1.0.9
  hypre: v2.33.0-23-g4dd96d0e8
  json: GIT-NOTFOUND
  libCEED: v0.13.0-rc.2-173-g95bd1e90
  libxsmm: 3469aa806
  magma: GIT-NOTFOUND
  metis: v5.1.0-p13
  mfem: v4.8-1012-g0c4c006ef8-dirty
  mumps: GIT-NOTFOUND
  parmetis: v4.0.3-p10
  petsc: v3.23.6-593-g0311516ef26
  scalapack: GIT-NOTFOUND
  scn: GIT-NOTFOUND
  slepc: v3.23.3-153-g61182f12f
  sundials: v7.4.0
  superlu_dist: v9.1.0-88-ge621c471-dirty

which is pretty cool. One thing is it could be good to discriminate between dependencies not built (strumpack and scalapack here for instance), and those without git like eigen and scn. Confirmed this by enabling strumpack and then it appears in the list.

Based on this, two more ideas:

1. For dependencies not built, don't print anything, rather than GIT-NOTFOUND
2. For a dependency which is present, but not git, report the version numbering like you did in your original version.

That way there'd be a full suite of information irrespective of the source, and nothing to mislead, it should also then play nice with spack, as it would only find mfem in the dependency dir and only report that.