-
Notifications
You must be signed in to change notification settings - Fork 21
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
Features required for building a Python-wrapped version of SHiELD #27
Comments
Hi, Spencer. Very glad that you could get this kicked off and made such
good progress in a short time.
Building FV3 as a library is a nice addition that would be useful for
purposes beyond just the SHiELD wrapper (it could conceivably be useful for
any FV3-based model), although I will leave it to Rusty whether this is the
best way to approach this. As you said we might need to wait until after
the shutdown to do a more thorough review.
Thanks,
Lucas
…On Fri, Sep 29, 2023 at 12:27 PM Spencer Clark ***@***.***> wrote:
*Is your feature request related to a problem? Please describe.*
At AI2 we have developed a Python-wrapped version of NOAA's FV3GFS model,
a close relative of SHiELD. The code is currently maintained here
<https://github.com/ai2cm/fv3gfs-fortran/tree/master/FV3/wrapper>, and
described in McGibbon et al. (2021)
<https://gmd.copernicus.org/articles/14/4401/2021/gmd-14-4401-2021.html>.
It has been a powerful tool that has facilitated several published hybrid
machine learning research projects. A drawback of it, however, is that
while similar to SHiELD, our now aging fork of FV3GFS is not the same, and
a number of features lag behind what has been developed and made public in
SHiELD. We would like to transition from wrapping FV3GFS to wrapping
SHiELD, both for our work at AI2 and for the fact that it would make it
more attractive (and intuitive) for users of SHiELD to use our
Python-wrapped model. It has been something on the roadmap of @lharris4
<https://github.com/lharris4> and our group for a while.
I have set up a repository for such a wrapper, which includes a PR
introducing a basic initial working version: ai2cm/SHiELD-wrapper#1
<ai2cm/SHiELD-wrapper#1>. This is not totally
feature-complete (as described in the PR description), but it represents a
major first step towards our eventual goal. In addition, as part of this
goal, we would like to move away from forking the entire fortran codebase
and instead depend directly on the evolving public repositories of the
NOAA-GFDL organization, which will keep the Python wrapper up to date. What
this means, however, is that any changes to the fortran code and build
system required to implement wrapper features must be made in these
repositories. This issue is meant to introduce this overall effort, and to
discuss how best to incorporate the initial required changes for wrapping
SHiELD in Python in a similar way to have we have wrapped FV3GFS.
*Initial required changes*
In this initial phase, the main changes required were to the SHiELD_build
repository (I made a stab at this in this branch
<main...spencerkclark:SHiELD_build:harmonize-library>,
but there may be another preferred way of going about it):
- We need to be able to build a static library containing the
dynamical core and atmos_drivers code, which we can link to when compiling
the wrapper.
- I did this by adding another config option for shield_wrapper,
which splits off all but the coupler_main.F90 file into static
libraries, before building the executable.
- We need a way to compile all pieces of the code (FMS, nceplibs, and
SHiELD code) as "position independent code," i.e. with the -fPIC flag.
- I did this by threading through the config option into the FMS,
nceplibs, and SHiELD build scripts, and add the -fPIC flag in the
event that config equals "shield_wrapper".
There was also a very minor change needed to declare some variables as
public in the atmos_drivers repository (see this small diff
<NOAA-GFDL/atmos_drivers@main...spencerkclark:atmos_drivers:public>
).
*Ask*
Please let me know from the SHiELD / SHiELD_build perspective if things
look reasonable enough for me to initiate pull requests from these branches
for review. There is no need to review ai2cm/SHiELD-wrapper#1
<ai2cm/SHiELD-wrapper#1>, though you are of
course welcome to take a look there for more context (I will wait to merge
that until I am no longer pointing to personal forks of the SHiELD_build
and atmos_drivers repositories).
I am happy to answer any questions about / discuss this project that come
up now or later. Thanks!
*Disclaimer*
I totally recognize I am posting this issue on the eve of a possible
government shutdown, and so I understand that you will likely not be able
to look deeply into this issue today (and possibly not for several days
into the future).
—
Reply to this email directly, view it on GitHub
<#27>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMUQRVD53JWIV56P6NXQEPDX43ZGBANCNFSM6AAAAAA5MWS3S4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Note that we will probably eventually need to support 32-bit/mixed-mode
arithmetic if we wish for greater uptake of the SHiELD wrapper, since we
typically run SHiELD with 32-bit dynamics for efficiency purposes.
On Fri, Sep 29, 2023 at 11:06 PM Lucas Harris - NOAA Federal <
***@***.***> wrote:
… Hi, Spencer. Very glad that you could get this kicked off and made such
good progress in a short time.
Building FV3 as a library is a nice addition that would be useful for
purposes beyond just the SHiELD wrapper (it could conceivably be useful for
any FV3-based model), although I will leave it to Rusty whether this is the
best way to approach this. As you said we might need to wait until after
the shutdown to do a more thorough review.
Thanks,
Lucas
On Fri, Sep 29, 2023 at 12:27 PM Spencer Clark ***@***.***>
wrote:
> *Is your feature request related to a problem? Please describe.*
>
> At AI2 we have developed a Python-wrapped version of NOAA's FV3GFS model,
> a close relative of SHiELD. The code is currently maintained here
> <https://github.com/ai2cm/fv3gfs-fortran/tree/master/FV3/wrapper>, and
> described in McGibbon et al. (2021)
> <https://gmd.copernicus.org/articles/14/4401/2021/gmd-14-4401-2021.html>.
> It has been a powerful tool that has facilitated several published hybrid
> machine learning research projects. A drawback of it, however, is that
> while similar to SHiELD, our now aging fork of FV3GFS is not the same, and
> a number of features lag behind what has been developed and made public in
> SHiELD. We would like to transition from wrapping FV3GFS to wrapping
> SHiELD, both for our work at AI2 and for the fact that it would make it
> more attractive (and intuitive) for users of SHiELD to use our
> Python-wrapped model. It has been something on the roadmap of @lharris4
> <https://github.com/lharris4> and our group for a while.
>
> I have set up a repository for such a wrapper, which includes a PR
> introducing a basic initial working version: ai2cm/SHiELD-wrapper#1
> <ai2cm/SHiELD-wrapper#1>. This is not totally
> feature-complete (as described in the PR description), but it represents a
> major first step towards our eventual goal. In addition, as part of this
> goal, we would like to move away from forking the entire fortran codebase
> and instead depend directly on the evolving public repositories of the
> NOAA-GFDL organization, which will keep the Python wrapper up to date. What
> this means, however, is that any changes to the fortran code and build
> system required to implement wrapper features must be made in these
> repositories. This issue is meant to introduce this overall effort, and to
> discuss how best to incorporate the initial required changes for wrapping
> SHiELD in Python in a similar way to have we have wrapped FV3GFS.
>
> *Initial required changes*
>
> In this initial phase, the main changes required were to the SHiELD_build
> repository (I made a stab at this in this branch
> <main...spencerkclark:SHiELD_build:harmonize-library>,
> but there may be another preferred way of going about it):
>
> - We need to be able to build a static library containing the
> dynamical core and atmos_drivers code, which we can link to when compiling
> the wrapper.
> - I did this by adding another config option for shield_wrapper,
> which splits off all but the coupler_main.F90 file into static
> libraries, before building the executable.
> - We need a way to compile all pieces of the code (FMS, nceplibs, and
> SHiELD code) as "position independent code," i.e. with the -fPIC
> flag.
> - I did this by threading through the config option into the FMS,
> nceplibs, and SHiELD build scripts, and add the -fPIC flag in the
> event that config equals "shield_wrapper".
>
> There was also a very minor change needed to declare some variables as
> public in the atmos_drivers repository (see this small diff
> <NOAA-GFDL/atmos_drivers@main...spencerkclark:atmos_drivers:public>
> ).
>
> *Ask*
>
> Please let me know from the SHiELD / SHiELD_build perspective if things
> look reasonable enough for me to initiate pull requests from these branches
> for review. There is no need to review ai2cm/SHiELD-wrapper#1
> <ai2cm/SHiELD-wrapper#1>, though you are of
> course welcome to take a look there for more context (I will wait to merge
> that until I am no longer pointing to personal forks of the SHiELD_build
> and atmos_drivers repositories).
>
> I am happy to answer any questions about / discuss this project that come
> up now or later. Thanks!
>
> *Disclaimer*
>
> I totally recognize I am posting this issue on the eve of a possible
> government shutdown, and so I understand that you will likely not be able
> to look deeply into this issue today (and possibly not for several days
> into the future).
>
> —
> Reply to this email directly, view it on GitHub
> <#27>, or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AMUQRVD53JWIV56P6NXQEPDX43ZGBANCNFSM6AAAAAA5MWS3S4>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
Thanks @lharris4, yes, it will be useful to get @bensonr's thoughts. 32-bit/mixed-mode support has indeed crossed my mind, though it will take some time to think about how to best support that in the wrapper, which we have sort of punted on up to this point with FV3GFS. My initial priority, however, will be on implementing the additional features necessary for @tlhsieh's work. |
Hi, Spencer. Thanks for your explanation. As a research tool 64-bit will be
fine but for broader uptake we will need mixed-mode. My (admittedly
limited) understanding is that this should be fairly straightforward in
python, but there are probably a lot of technical specifics I am glossing
over.
Lucas
…On Mon, Oct 2, 2023 at 8:09 AM Spencer Clark ***@***.***> wrote:
Thanks @lharris4 <https://github.com/lharris4>, yes, it will be useful to
get @bensonr <https://github.com/bensonr>'s thoughts.
32-bit/mixed-mode support has indeed crossed my mind, though it will take
some time to think about how to best support that in the wrapper, which we
have sort of punted on up to this point with FV3GFS. My initial priority,
however, will be on implementing the additional features necessary for
@tlhsieh <https://github.com/tlhsieh>'s work.
—
Reply to this email directly, view it on GitHub
<#27 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMUQRVCCPR6T3PPR6Z4IYELX5KVINAVCNFSM6AAAAAA5MWS3S6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONBSHA4TSMBTHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Understood -- I created ai2cm/SHiELD-wrapper#2 to track further discussion on that topic, since it should not have any bearing on the initial discussion here. |
Back to the point from @spencerkclark and @lharris4 regarding FV3 as a library. The typical way FRE compiles up models is to build each component as a static library and link them in when compiling up the driver program (in this case coupler_main.F90). If you have a PR that does this in general and not just for your specific project, I think it would be a welcome update to the SHiELD_build system. |
Thanks @bensonr -- that makes sense. Per our discussion yesterday, I made a couple PRs related to these changes, which should be ready for your and / or @laurenchilutti's review when you get a chance: |
Is your feature request related to a problem? Please describe.
At AI2 we have developed a Python-wrapped version of NOAA's FV3GFS model, a close relative of SHiELD. The code is currently maintained here, and described in McGibbon et al. (2021). It has been a powerful tool that has facilitated several published hybrid machine learning research projects. A drawback of it, however, is that while similar to SHiELD, our now aging fork of FV3GFS is not the same, and a number of features lag behind what has been developed and made public in SHiELD. We would like to transition from wrapping FV3GFS to wrapping SHiELD, both for our work at AI2 and for the fact that it would make it more attractive (and intuitive) for users of SHiELD to use our Python-wrapped model. It has been something on the roadmap of @lharris4 and our group for a while.
I have set up a repository for such a wrapper, which includes a PR introducing a basic initial working version: ai2cm/SHiELD-wrapper#1. This is not totally feature-complete (as described in the PR description), but it represents a major first step towards our eventual goal. In addition, as part of this goal, we would like to move away from forking the entire fortran codebase and instead depend directly on the evolving public repositories of the NOAA-GFDL organization, which will keep the Python wrapper up to date. What this means, however, is that any changes to the fortran code and build system required to implement wrapper features must be made in these repositories. This issue is meant to introduce this overall effort, and to discuss how best to incorporate the initial required changes for wrapping SHiELD in Python in a similar way to how we have wrapped FV3GFS.
Initial required changes
In this initial phase, the main changes required were to the SHiELD_build repository (I made a stab at this in this branch, but there may be another preferred way of going about it):
config
option forshield_wrapper
, which splits off all but thecoupler_main.F90
file into static libraries, before building the executable.-fPIC
flag.config
option into the FMS, nceplibs, and SHiELD build scripts, and adding the-fPIC
flag in the event thatconfig
equals"shield_wrapper"
.There was also a very minor change needed to declare some variables as public in the atmos_drivers repository (see this small diff).
Ask
Please let me know from the SHiELD / SHiELD_build perspective if things look reasonable enough for me to initiate pull requests from these branches for review. There is no need to review ai2cm/SHiELD-wrapper#1, though you are of course welcome to take a look there for more context (I will wait to merge that until I am no longer pointing to personal forks of the SHiELD_build and atmos_drivers repositories).
I am happy to answer any questions about / discuss this project that come up now or later. Thanks!
Disclaimer
I totally recognize I am posting this issue on the eve of a possible government shutdown, and so I understand that you will likely not be able to look deeply into this issue today (and possibly not for several days into the future).
The text was updated successfully, but these errors were encountered: