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

gcm_setup python conversion #602

Draft
wants to merge 17 commits into
base: develop
Choose a base branch
from
Draft

Conversation

sshakoor1
Copy link
Contributor

@sshakoor1 sshakoor1 commented Apr 9, 2024

The purpose of this PR is to fully rewrite gcm_setup in python, as a small start of a larger effort to refactor the GEOS codebase.

Some notes:

  • Currently, only uncoupled AMIP experiments are zero diff from cshell
  • gcm_setup has been uncharacteristically edited/expanded upon quite a bit this past year, so a lot of features have yet to be added (e.g. singularity sandbox, data atmosphere, etc.). In addition, some parts of the code may be outdated from the latest commit of the mainline script.
  • jinja2 templating is being used, and in order for it to work jinja's delimiter syntax needed to implemented into all the template files. This means that all variables in gcm_setup that previously had the syntax @foo now look like {{ foo }}.
  • Cloning experiments is still a work in progress and is not currently an option. In the future it will require passing in yaml files created from previous experiments.
  • While this PR attempts to heavily refactor the code, it is not perfect, and probably needs to be refactored down the line itself.
  • Can only be run on python 3.11 and higher. To run, simply navigate to ~\GEOSgcm\install\bin\gcmpy\scripts\ and run the command python gcm_setup.py

@sshakoor1 sshakoor1 self-assigned this Apr 9, 2024
@sshakoor1 sshakoor1 requested review from tclune and mathomp4 April 9, 2024 20:32
@sshakoor1 sshakoor1 added 0 diff The changes in this pull request have verified to be zero-diff with the target branch. 0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) 0 diff structural Structural changes to repository that are zero-diff labels Apr 9, 2024
@sshakoor1 sshakoor1 removed the 0 diff The changes in this pull request have verified to be zero-diff with the target branch. label Apr 9, 2024
gcmpy/CMakeLists.txt Outdated Show resolved Hide resolved
gcmpy/jinjafy.py Outdated
@@ -0,0 +1,257 @@
'''
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering if there is a better name for this file. The filename should reflect what it is used for - not what technology it uses.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly this file should not be included in this PR, it is mainly a helper script for me. No one else should need to ever run this.

@@ -0,0 +1,67 @@
import os, sys, platform
Copy link
Collaborator

Choose a reason for hiding this comment

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

"utility" is a bit vague. Possibly a more specific name could be thought of?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I agree. Env and utility should probably be merged into a new script down the line. These scripts were kind of thrown together early on and desperately need to be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff structural Structural changes to repository that are zero-diff 0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants