Updating submodules and pace refactor

[fmalatino]

Description This PR updates the NDSL, pyFV3, and pySHiELD submodules to most recent commits, reflecting the standardized import methods.

How Has This Been Tested? Tested using the currently implemented unit tests in tests/main

Checklist:

• [x] My code follows the style guidelines of this project
• [x] I have performed a self-review of my own code
• [x] I have commented my code, particularly in hard-to-understand areas
• [x] I have made corresponding changes to the documentation
• [x] My changes generate no new warnings
• [x] Any dependent changes have been merged and published in downstream modules
• [x] New check tests, if applicable, are included

[fmalatino]

As I look at the driver/pace/driver code I have a question about what we are building with Pace. If Pace is designed to be the model and we will be adding other components (land, ice, ocean, waves, etc.) within, I'm not sure it makes senes to treat the driver as a submodule. However, if we envision Pace being say the atmospheric component within a larger module, it does make sense to leave the submodule treatment in place.

@oelbert ?

[oelbert]

My understanding has been that Pace is solely an atmospheric model and any ocean or land model would be separate

[lharris4]

Currently Pace is intended to be an atmosphere model, possibly containing a land component. Ocean and ice are separate from Pace's scope.

Lucas

On Tue, Apr 9, 2024 at 5:01 PM Oliver Elbert @.***> wrote:

My understanding has been that Pace is solely an atmospheric model and any ocean or land model would be separate

— Reply to this email directly, view it on GitHub https://github.com/NOAA-GFDL/pace/pull/75#issuecomment-2046039833, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMUQRVBBT3NTRO5LWX5ZW5DY4RJKDAVCNFSM6AAAAABFCCM2UOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBWGAZTSOBTGM . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[oelbert]

The driver/pace structure still seems redundant to me. Perhaps it is my lack of Python semantics, but we are in the Pace repo and have a driver/. Why do I need to yet again remind you this is pace by having driver/pace/? Is it better to have pace/ or pace/driver so that all references are pace.X or pace.driver.X?

pace/driver/pace/driver/* structure is definitely a relic of Pace's development history, and we want to resolve that. Do you think we should do it here or in a separate PR?

[bensonr]

In response to this comment, this PR is already renaming things from driver/pace/driver. Instead of doing it yet again, I was hoping we could get the structure set in one step. As always, I want the approach with the least chance of breaking things.

[fmalatino]

In response to this comment, this PR is already renaming things from driver/pace/driver. Instead of doing it yet again, I was hoping we could get the structure set in one step. As always, I want the approach with the least chance of breaking things.

Currently the method of setting up the pace package differs from its submodules in that it uses a requirements file for specifying the requirements for the pip install of the package, as opposed to passing them through a setup.py or other build backend method. Within the requirements file we specify a need for an editable install of the submodules and pace, which will search for a setup.py file in each of the directories specified. Each of the setup.py files look for a namespace package matching the specified name(s) in the call to the find_namespace_packages. These namespace packages must be contained within a directory alongside the setup.py file. Proposed in PR:

driver/
├── pace/      
│   ├── __init__.py      
│   └── driver.py
│   └── other.py            
└── setup.py

Current:

driver/
├── pace/ 
│     ├── driver/      
│          ├── __init__.py      
│          └── driver.py
│          └── other.py            
└── setup.py

I chose driver to serve as the outer directory to contain the pace package and the setup.py with the intention that once installed the user would then use the package like:

import pace

driver = pace.Driver(...)

As opposed to what existed previously:

import pace.driver

driver = pace.driver.Driver(...)

We can change the use of setup.py to include all of the desired requirements and make calls to install the submodules as well, which will then treat the cloned repo as the outer directory containing the pace package alongside its setup.py:

cloned_pace/
├── pace/      
│   ├── __init__.py      
│   └── driver.py
│   └── other.py            
└── setup.py
└── other_mods/

This will however not use the requirements file method, and will require some minor changes elsewhere, like in the github workflows, etc. I have tested both of these methods and went with the former as it keeps prevents a need to change the other files. If the latter is preferred the changes can be easily made. Also, I am not attached to the name driver for the outer directory, only using it as it made it easy to implement.

[fmalatino]

@bensonr and @oelbert

I have made changes to the requirements_dev.txt file which should allow for pace to be installed without an outer directory (aside from it being contained in the cloned repo).