Closed spencerkclark closed 1 year ago
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 https://github.com/ai2cm/SHiELD-wrapper/pull/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 https://github.com/NOAA-GFDL/SHiELD_build/compare/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 https://github.com/NOAA-GFDL/atmos_drivers/compare/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 https://github.com/ai2cm/SHiELD-wrapper/pull/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 https://github.com/NOAA-GFDL/SHiELD_build/issues/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 https://github.com/ai2cm/SHiELD-wrapper/pull/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 https://github.com/NOAA-GFDL/SHiELD_build/compare/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 https://github.com/NOAA-GFDL/atmos_drivers/compare/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 https://github.com/ai2cm/SHiELD-wrapper/pull/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 https://github.com/NOAA-GFDL/SHiELD_build/issues/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 https://github.com/NOAA-GFDL/SHiELD_build/issues/27#issuecomment-1742899039, 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: https://github.com/ai2cm/SHiELD-wrapper/pull/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 https://github.com/ai2cm/SHiELD-wrapper/pull/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).