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

Feature/wdboggs/use acg #6

Merged
merged 35 commits into from
Dec 22, 2022
Merged

Feature/wdboggs/use acg #6

merged 35 commits into from
Dec 22, 2022

Conversation

darianboggs
Copy link
Contributor

Began implementation of ACG in GEOS_OceanGridComp

@darianboggs darianboggs requested review from a team as code owners June 13, 2022 17:58
tclune
tclune previously approved these changes Jun 13, 2022
Copy link
Contributor

@tclune tclune left a comment

Choose a reason for hiding this comment

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

CMake changes ok.

@yvikhlya
Copy link

Will this be automatically tested for zero diff in coupled mode, or do we still need to run a zero diff test manually?

@sanAkel sanAkel added the enhancement New feature or request label Jun 13, 2022
@sanAkel
Copy link
Collaborator

sanAkel commented Jun 14, 2022

fixes #6

@sanAkel
Copy link
Collaborator

sanAkel commented Jun 14, 2022

There are multiple errors when building, please see ⬇️

@tclune I will be in touch to get help.

@darianboggs, you can ignore this message or if you know how to fix, please commit. Thanks!

Scanning dependencies of target GEOS_OceanGridComp
[ 86%] Building Fortran object src/Components/@GEOSgcm_GridComp/GEOSogcm_GridComp/@GEOS_OceanGridComp/CMakeFiles/GEOS_OceanGridComp.dir/GEOS_OceanGridComp.F90.o
./GEOS_Ocean_DeclarePointer___.h(37): error #6417: The dimensions of this array have been defined more than once. [TAUX]
real, pointer, dimension(:,:) :: TAUX => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(37): error #6227: This symbol has multiple POINTER statement/attribute declarations which is not allowed. [TAUX]
real, pointer, dimension(:,:) :: TAUX => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(38): error #6417: The dimensions of this array have been defined more than once. [TAUY]
real, pointer, dimension(:,:) :: TAUY => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(38): error #6227: This symbol has multiple POINTER statement/attribute declarations which is not allowed. [TAUY]
real, pointer, dimension(:,:) :: TAUY => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(39): error #6417: The dimensions of this array have been defined more than once. [SWHEAT]
real, pointer, dimension(:,:,:) :: SWHEAT => null()
--------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(39): error #6227: This symbol has multiple POINTER statement/attribute declarations which is not allowed. [SWHEAT]
real, pointer, dimension(:,:,:) :: SWHEAT => null()
--------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(41): error #6417: The dimensions of this array have been defined more than once. [DISCHARGE]
real, pointer, dimension(:,:) :: DISCHARGE => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(41): error #6227: This symbol has multiple POINTER statement/attribute declarations which is not allowed. [DISCHARGE]
real, pointer, dimension(:,:) :: DISCHARGE => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(42): error #6417: The dimensions of this array have been defined more than once. [FROCEAN]
real, pointer, dimension(:,:) :: FROCEAN => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(42): error #6227: This symbol has multiple POINTER statement/attribute declarations which is not allowed. [FROCEAN]
real, pointer, dimension(:,:) :: FROCEAN => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(43): error #6417: The dimensions of this array have been defined more than once. [LWFLX]
real, pointer, dimension(:,:) :: LWFLX => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(43): error #6227: This symbol has multiple POINTER statement/attribute declarations which is not allowed. [LWFLX]
real, pointer, dimension(:,:) :: LWFLX => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(45): error #6417: The dimensions of this array have been defined more than once. [SHFLX]
real, pointer, dimension(:,:) :: SHFLX => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(45): error #6227: This symbol has multiple POINTER statement/attribute declarations which is not allowed. [SHFLX]
real, pointer, dimension(:,:) :: SHFLX => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(46): error #6417: The dimensions of this array have been defined more than once. [QFLUX]
real, pointer, dimension(:,:) :: QFLUX => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(46): error #6227: This symbol has multiple POINTER statement/attribute declarations which is not allowed. [QFLUX]
real, pointer, dimension(:,:) :: QFLUX => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(48): error #6417: The dimensions of this array have been defined more than once. [RAIN]
real, pointer, dimension(:,:) :: RAIN => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(48): error #6227: This symbol has multiple POINTER statement/attribute declarations which is not allowed. [RAIN]
real, pointer, dimension(:,:) :: RAIN => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(49): error #6417: The dimensions of this array have been defined more than once. [SNOW]
real, pointer, dimension(:,:) :: SNOW => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(49): error #6227: This symbol has multiple POINTER statement/attribute declarations which is not allowed. [SNOW]
real, pointer, dimension(:,:) :: SNOW => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(50): error #6417: The dimensions of this array have been defined more than once. [PEN_OCN]
real, pointer, dimension(:,:) :: PEN_OCN => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(50): error #6227: This symbol has multiple POINTER statement/attribute declarations which is not allowed. [PEN_OCN]
real, pointer, dimension(:,:) :: PEN_OCN => null()
------------------------------------^
/discover/nobackup/sakella/geosMom6/acg_WB/GEOSgcm/src/Components/@GEOSgcm_GridComp/GEOSogcm_GridComp/@GEOS_OceanGridComp/GEOS_OceanGridComp.F90(952): error #6417: The dimensions of this array have been defined more than once. [FRZMLT]
real, pointer :: FRZMLT(:,:)
---------------------^
/discover/nobackup/sakella/geosMom6/acg_WB/GEOSgcm/src/Components/@GEOSgcm_GridComp/GEOSogcm_GridComp/@GEOS_OceanGridComp/GEOS_OceanGridComp.F90(952): error #6227: This symbol has multiple POINTER statement/attribute declarations which is not allowed. [FRZMLT]
real, pointer :: FRZMLT(:,:)
---------------------^
./GEOS_Ocean_DeclarePointer___.h(12): error #7158: Array attributes for this symbol occurred after the data initializers. [FROCEAN]
real, pointer, dimension(:,:) :: FROCEAN => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(12): error #7367: The data value NULL() can only be assigned to a Fortran POINTER.
real, pointer, dimension(:,:) :: FROCEAN => null()
-----------------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(13): error #7158: Array attributes for this symbol occurred after the data initializers. [TAUX]
real, pointer, dimension(:,:) :: TAUX => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(13): error #7367: The data value NULL() can only be assigned to a Fortran POINTER.
real, pointer, dimension(:,:) :: TAUX => null()
--------------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(14): error #7158: Array attributes for this symbol occurred after the data initializers. [TAUY]
real, pointer, dimension(:,:) :: TAUY => null()
------------------------------------^
./GEOS_Ocean_DeclarePointer___.h(14): error #7367: The data value NULL() can only be assigned to a Fortran POINTER.
real, pointer, dimension(:,:) :: TAUY => null()
--------------------------------------------^
GEOS_OceanGridComp.i90(2166): catastrophic error: Too many errors, exiting

@darianboggs
Copy link
Contributor Author

I'm looking at the errors. @tclune @sanAkel

@sanAkel
Copy link
Collaborator

sanAkel commented Jun 15, 2022

I'm looking at the errors. @tclune @sanAkel

Please push any changes you may have ... I plan to work on this tomorrow. Thanks!

@darianboggs
Copy link
Contributor Author

I think the problem is related to certain fields being both Import and Export. ACG generates pointer declarations for ALL the GridSpec's. So that means that it generates declarations for some fields twice. I restored the pointer declarations and MAPL_GetPointer calls and removed the #include lines for pointer declarations and MAPL_GetPointer calls. @sanAkel @tclune

I committed changes, and I am compiling now. I'll let you know how it turns out.

@sanAkel
Copy link
Collaborator

sanAkel commented Jun 16, 2022

That's true! We have some silly stuff going on.

I can take care of it by renaming the variables and dealing with the exports. It's up to you to leave it there for me.

Thanks for the help!

@darianboggs
Copy link
Contributor Author

Great! It compiled successfully. ACG has a narrow focus currently. So doesn't handle a case like yours fully, though it might be a valid case. Nonetheless, you do get the benefit of it generating most of the MAPL_AddSpec calls for you. The ...GetPointer____.h and ...DeclarePointer___.h will still be created by ACG, but they won't be included in your F90 file.

So, you may not need or want to change the variables. ACG generates pointer declarations and MAPL_GetPointer calls based on the short name. Your choice.

@tclune
Copy link
Contributor

tclune commented Jun 16, 2022

We definitely need @atrayano to comment on the deeper issue here when he's back online.

@sanAkel
Copy link
Collaborator

sanAkel commented Jun 16, 2022

@darianboggs, Thanks for the modifications. It builds for me too.
I'll test/take your changes in children GCs: MOM6_GEOSPlug.F90 and MOM_GEOS5PlugMod.F90. As for the parent, I think it can be cleaned up to take full advantage of the ACG. I'll try ...

Thanks again!

@github-actions
Copy link

Label error. Requires at least 1 of: 0 diff, 0 diff trivial, Non 0-diff, 0 diff structural, 0-diff trivial, Not 0-diff, 0-diff, automatic, 0-diff uncoupled. Found: enhancement

@sanAkel sanAkel added the 0 diff structural Structural changes to repository that are zero-diff label Dec 15, 2022
…o FRACICE for ACG to work. You can check auto-generated code at: src/Components/@GEOSgcm_GridComp/GEOSogcm_GridComp/@GEOS_OceanGridComp/GEOSdatasea_GridComp/
…OM6_GEOSPlug_DeclarePointer___.h and MOM6_GEOSPlug_GetPointer___.h are NOT being used right now, because a few variables are dummy imports (e.g., SWHEAT) and exports (e.g, DH, T, S) from MOM6 ocean model. Unless these dummy imports and exports disappear and connectivity is CLEANED up, using these ACG files is almost impossible.Someday when MOM5 is deprecated, revisit.
… marked as DUMMY_EXPORT, they are there only because of messy connectivities.
…ices, can not use Declare and Get Pointers because same names are used (twice) to import and export- ACG creates statements twice, so it cannot used unless these imports and exports are RENAMED. If they are renamed- NOW, all connectivity gets messed up- so do it another day
@sanAkel
Copy link
Collaborator

sanAkel commented Dec 22, 2022

ACG is now implemented across the board in:

  1. Data sea
  2. MOM5 coupler or plug
  3. MOM6 coupler or plug

Answers are preserved in:

  • ✅ Data sea mode
  • ✅ MOM6 ocean; Given the scope of this PR, NO change in results is expected with MOM5 as well.

Significant amount of tidying up has been done as well.

Copy link
Collaborator

@sanAkel sanAkel left a comment

Choose a reason for hiding this comment

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

@darianboggs, @mathomp4, This is good to go now. If you feel like, please take a look. All tests passed. ✅

@mathomp4, You probably need to use super powers to merge it! 😄

@mathomp4 mathomp4 merged commit b171e3f into develop Dec 22, 2022
@mathomp4
Copy link
Member

If you're happy, I'm happy. I'll merge in now.

I suppose you should soon make a PR to GEOSgcm so we can get the CI working again in here...

@mathomp4 mathomp4 deleted the feature/wdboggs/use_ACG branch December 22, 2022 14:22
@darianboggs
Copy link
Contributor Author

darianboggs commented Dec 22, 2022 via email

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 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants