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

Replace Castro microphysics with starkiller-astro Microphysics #760

Merged
merged 19 commits into from
Feb 11, 2020

Conversation

maxpkatz
Copy link
Member

@maxpkatz maxpkatz commented Feb 9, 2020

PR summary

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.

For Castro developers, this means that -- at a minimum -- we must update the Microphysics submodule monthly when we issue new releases. The release for Microphysics must be done first. Then navigate to the Microphysics directory, checkout the new tag, and then from the top-level directory of Castro do a git add on the Microphysics directory to store the new tag. So, for example, at the beginning of March 2020 we would first issue the 20.03 tag on Microphysics, then do

cd $CASTRO_HOME/Microphysics
git pull
git checkout 20.03
cd ..
git add Microphysics
git commit -m "Update Microphysics to release 20.03"

Then we can proceed with issuing our own release.

When breaking changes to Microphysics occur in its development branch that Castro depends on, we must update the Microphysics submodule on the Castro development branch in the same way, replacing the git checkout statement with the latest commit hash on the Microphysics development branch. A git submodule always tracks a specific commit/tag on the target repo -- it is not configured to automatically track a particular branch.

In order to ensure that we don't lose track of this coupling between the two codebases, it is my recommendation that we use this git submodule for the Castro regression test suite, rather than relying on a standalone installation of Microphysics. So we should remove any references to MICROPHYSICS_HOME as part of that setup, and then make sure the git commands are appropriately updated.

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.

PR checklist

  • test suite needs to be run on this PR
  • this PR will change answers in the test suite
  • all functions have docstrings as per the coding conventions
  • the CHANGES file has been updated
  • if appropriate, this change is described in the docs

@maxpkatz maxpkatz marked this pull request as ready for review February 10, 2020 02:09
@maxpkatz maxpkatz mentioned this pull request Feb 10, 2020
5 tasks
@zingale
Copy link
Member

zingale commented Feb 11, 2020

note: this also implements #690

@zingale
Copy link
Member

zingale commented Feb 11, 2020

all tests pass here, using the current external Microphysics.

I'm going to merge and then change up the test suite shortly. I think we want to test both ways -- using submodules (which is how users will probably use the code, but likely on master) and using the separate Microphysics (which is how developers will likely use the code).

@zingale zingale merged commit f08a875 into development Feb 11, 2020
@zingale zingale deleted the microphysics branch February 11, 2020 19:12
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.

2 participants