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

ACG Rebuild Issue again in GEOS: datasea (and Ocean?) #1912

Closed
mathomp4 opened this issue Jan 9, 2023 · 4 comments · Fixed by #1916
Closed

ACG Rebuild Issue again in GEOS: datasea (and Ocean?) #1912

mathomp4 opened this issue Jan 9, 2023 · 4 comments · Fixed by #1916
Assignees
Labels
🪲 Bugfix This fixes a bug!

Comments

@mathomp4
Copy link
Member

mathomp4 commented Jan 9, 2023

This is a bit of a repeat of the issue solved by #1448. In doing some GOCART tests today, I notice that if you do a no-op rebuild of GEOS you get:

[ 72%] Building Fortran object src/Components/@GEOSgcm_GridComp/GEOSogcm_GridComp/@GEOS_OceanGridComp/GEOSdatasea_GridComp/CMakeFiles/GEOSdatasea_GridComp.dir/GEOS_DataSeaGridComp.F90.o

I mean, I didn't go near datasea. But that is a component that @sanAkel worked with @darianboggs on using the ACG. So something is a bit wonky. Thing is datasea calls the ACG:

mapl_acg (${this}   GEOS_DataSea_StateSpecs.rc
          IMPORT_SPECS EXPORT_SPECS INTERNAL_SPECS
          GET_POINTERS DECLARE_POINTERS)

the same as, say, dust:

mapl_acg (${this}   DU2G_StateSpecs.rc
          IMPORT_SPECS EXPORT_SPECS INTERNAL_SPECS
          GET_POINTERS DECLARE_POINTERS)

Thus I suspect the issue is with the CMake macro/function in MAPL.

@mathomp4 mathomp4 added the 🪲 Bugfix This fixes a bug! label Jan 9, 2023
@mathomp4 mathomp4 self-assigned this Jan 9, 2023
@mathomp4
Copy link
Member Author

mathomp4 commented Jan 9, 2023

Okay. I think I know the issue. If we look, per the CMake code, it is saying the files generated will be:

-- mapl_acg() - generated files: GEOSdatasea_Import___.h;GEOSdatasea_Export___.h;GEOSdatasea_Internal___.h;GEOSdatasea_GetPointer___.h;GEOSdatasea_DeclarePointer___.h

but the files we make are:

$ fd -e h datasea
src/Components/@GEOSgcm_GridComp/GEOSogcm_GridComp/@GEOS_OceanGridComp/GEOSdatasea_GridComp/GEOS_DataSea_Import___.h
src/Components/@GEOSgcm_GridComp/GEOSogcm_GridComp/@GEOS_OceanGridComp/GEOSdatasea_GridComp/GEOS_DataSea_GetPointer___.h
src/Components/@GEOSgcm_GridComp/GEOSogcm_GridComp/@GEOS_OceanGridComp/GEOSdatasea_GridComp/GEOS_DataSea_DeclarePointer___.h
src/Components/@GEOSgcm_GridComp/GEOSogcm_GridComp/@GEOS_OceanGridComp/GEOSdatasea_GridComp/GEOS_DataSea_Export___.h
src/Components/@GEOSgcm_GridComp/GEOSogcm_GridComp/@GEOS_OceanGridComp/GEOSdatasea_GridComp/GEOS_DataSea_Internal___.h

So, the CMake thinks we'll make GEOSdatasea_Import___.h and we actually make GEOS_DataSea_Import___.h.

I'm pretty sure this is my fault. Before the ocean ACG code, we "accidentally" used similar names for both the component (aka ${this}) and the specs file:

DU2G_GridComp/DU2G_StateSpecs.rc

So what my code in #1448 does is essentially go off of the component name and use that to construct what we think the ACG will make. That is dumb. The Python ACG DOES NOT CARE about the component! All it takes is the StateSpecs file and removes .rc and then removes _StateSpecs and _Registry from that file name.

@sanAkel and @darianboggs have the file:

GEOSdatasea_GridComp/GEOS_DataSea_StateSpecs.rc

which is a good file and it works, but the CMake code is dumb and assumed the StateSpecs file would be called GEOSdatasea_StateSpecs.rc when there is no reason it should!

We just need to do the same algorithm in CMake that Python does in the ACG. I can code this up!

@sanAkel
Copy link

sanAkel commented Jan 9, 2023

@mathomp4,

What do you mean by no-op:

This is a bit of a repeat of the issue solved by #1448. In doing some GOCART tests today, I notice that if you do a no-op rebuild of GEOS you get:

Indeed I have it this way:

mapl_acg (${this} GEOS_DataSea_StateSpecs.rc
IMPORT_SPECS EXPORT_SPECS INTERNAL_SPECS
GET_POINTERS DECLARE_POINTERS)

But I really do not have internal_specs so I can take that away and the two are different -- though that underlying problem (with MAPL/ACG) remains? I'm not entire sure of what exactly the problem is (will try to talk to you).

@tclune
Copy link
Collaborator

tclune commented Jan 9, 2023

In case @sanAkel has not yet spoken with @mathomp4 . The issue is that if you type make again after a successful build, some things are recompiled, and this is not ideal. It is a minor error in our cmake logic that is worth fixing for sanity's sake.

@sanAkel
Copy link

sanAkel commented Jan 9, 2023

Thanks @tclune I plan to talk to @mathomp4 sometime soon ... but yes, nice to have that (recompilation) taken away!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪲 Bugfix This fixes a bug!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants