-
Notifications
You must be signed in to change notification settings - Fork 98
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
Make AMReX and Microphysics submodules of Castro #651
Comments
tl;dr:
It would be hard to come up with a more textbook use-case for git submodules. I support this idea. As someone that has used submodules in my repos, I would add that in addition to preparing users and the build system for initializing submodules, you also need to prepare both for regular updating of submodules. Every upstream update of a submodule requires all pullers of the parent code to manually update their version of the submodule, unless we can build some robust machinery into git hooks and/or the build system or we make the very dubious assumption that everyone will always do Submodule repos will also often be on detached heads, which to me I thought was an error but is natural for how submodules are intended to be used. Users should be prepared for this in case they're new to submodules. I think there are some tricks to try to keep submodules on Finally, by default
This article was very useful to me in my frustration with submodules, and could provide some source material for our documentation or any submodule automation we want to implement: https://medium.com/@porteneuve/mastering-git-submodules-34c65e940407 |
This implements the Microphysics part of #651. We replace the local Castro Microphysics directory with a dependence on the main starkiller-astro Microphysics directory. This means that obtaining Microphysics is now a requirement for using Castro. If the user sets MICROPHYSICS_HOME as before, then their workflow does not change. A new path is that Microphysics is a git submodule of Castro, so if the user does git submodule update --init --recursive then they will obtain the version of Microphysics that we specify in the repo, and if they do not have MICROPHYSICS_HOME set then this submodule will be used. The repo is now updated with git pull --recurse-submodules, so that updates to Microphysics come along when you pull. A minor associated change is that we've standardized on NETWORK_INPUTS in all of the makefiles, instead of using GENERAL_NET_INPUTS with the hardcoded path.
AMReX and Microphysics should be git submodules of Castro for a lower barrier to entry for new users.
I've sketched out what this looks like in mini-Castro already, for AMReX. We would create directories called amrex and Microphysics in the top-level Castro directory. Then inside those we set a specific commit of each of those codes to check out when you download Castro (which would presumably be the most recent monthly release of each of those codes -- this enforcement of version parity is a side benefit of going this route). A new user getting the code for the first time just has to do
and a user that already has the Castro code just has to do
to get the submodules downloaded.
This will not break the workflow of those of us who maintain those dependencies separately, since in the Makefile we will do
and if you already have
AMREX_HOME
in your environment it will pick up your other copy of it.We discussed this a few years ago and decided not to do this, but we should revisit this because my proposed mechanism gives a person the freedom to use the code either way. Also, since it is now very common for large HPC codes to have git submodule dependencies like this now (e.g. most Kokkos- and RAJA-based codes use Kokkos and RAJA as git submodules), I don't think this will be a confusing workflow for users.
A major benefit of this approach for Microphysics is that we would not have to maintain redundant copies of the Microphysics interfaces (and the gamma law EOS) in Castro's Microphysics directory. We could just have Microphysics supply that directly. I recall an argument that was made at the time was that we don't want users to be confused about extra requirements, but aside from my point above that I think most users are familiar with this idea by now, we can also have this be friendly by detecting if either MICROPHYSICS_HOME is set or if the Microphysics directory doesn't yet exist in the top-level directory (because they forgot to do git clone --recursive) and then printing out a helpful error message telling them what to do in that case. I've done this in mini-Castro for AMReX, as follows:
The text was updated successfully, but these errors were encountered: