CMake changes for ASPECT DebugRelease mode #686

tjhei · 2024-02-28T23:24:28Z

This PR adds a number of CMake options to rename the name of the gwb library and to disable certain targets/features. This change allows us to configure a debug and a release mode build of GWB when ASPECT is configured as DebugRelease mode.

This PR should not change anything in the default configuration.

We incorrectly always only built the debug version of an external GWB version 0.6 or newer if ASPECT was in DebugRelease mode. With GeodynamicWorldBuilder/WorldBuilder#686 we can now configure GWB twice inside the build directory of ASPECT and link to the correct library.

github-actions · 2024-02-28T23:42:29Z

Benchmark	Main	Feature	Difference (99.9% CI)
Slab interpolation simple none	1.018 ± 0.005 (s=443)	1.015 ± 0.004 (s=445)	-0.5% .. -0.3%
Slab interpolation curved simple none	1.014 ± 0.003 (s=451)	1.017 ± 0.003 (s=438)	+0.2% .. +0.3%
Spherical slab interpolation simple none	1.166 ± 0.005 (s=376)	1.167 ± 0.005 (s=398)	-0.0% .. +0.2%
Slab interpolation simple curved CMS	1.066 ± 0.005 (s=413)	1.067 ± 0.004 (s=433)	+0.1% .. +0.3%
Spherical slab interpolation simple CMS	1.550 ± 0.011 (s=302)	1.549 ± 0.009 (s=281)	-0.2% .. +0.1%
Spherical fault interpolation simple none	1.173 ± 0.007 (s=398)	1.172 ± 0.006 (s=372)	-0.2% .. +0.0%
Cartesian min max surface	2.309 ± 0.022 (s=194)	2.305 ± 0.015 (s=199)	-0.5% .. +0.1%
Spherical min max surface	7.213 ± 0.054 (s=60)	7.200 ± 0.038 (s=67)	-0.6% .. +0.2%

MFraters · 2024-02-29T02:57:50Z

CMakeLists.txt

@@ -528,7 +551,7 @@ if(MAKE_PYTHON_WRAPPER)
    endif()
  endif()

-  set_target_properties(${TARGET} PROPERTIES PREFIX "_" OUTPUT_NAME "gwb")
+  set_target_properties(${WB_TARGET} PROPERTIES PREFIX "_" OUTPUT_NAME "gwb")


This change seem sot be tripping it up. Note that ${TARGET} is not equal to WorldBuilder but to either _gwb or gwb.

The main problem is that the c/cpp/fortran examples expect a library called libWorldBuilder.a, which with this change is no longer generated, but it instead generated _gwb.a. Commenting out the line seems to let the testers pass on my system. I think this line may have been faulty an not executed.

codecov · 2024-02-29T05:04:20Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.31%. Comparing base (786dc8a) to head (51d965f).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #686      +/-   ##
==========================================
+ Coverage   98.30%   98.31%   +0.01%     
==========================================
  Files         107      107              
  Lines        7358     7358              
==========================================
+ Hits         7233     7234       +1     
+ Misses        125      124       -1

see 1 file with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 786dc8a...51d965f. Read the comment docs.

tjhei · 2024-02-29T14:13:41Z

okay, seems to work now. Thank you, @MFraters . The script is somewhat messy, but I don't know of a better way to make this work within ASPECT. What do you think?
The only alternative I can think of is that you also support a DebugRelease mode (similar to deal.II where two libary files are produced). This is not a very typical thing to do and maybe not worth the headache.

MFraters

Thanks for your work on this @tjhei! Although supporting DebugRelease in the future would be great, I don't think it needs to be done now.

I have two small comments on the new cmake variables, but otherwise it looks good to me.

MFraters · 2024-02-29T14:22:14Z

CMakeLists.txt

+set(WB_ENABLE_APPS ON CACHE BOOL "If enabled, applications gwb-grid and gwb-dat are built.")
+set(WB_ENABLE_HELPER_TARGETS ON CACHE BOOL "If enabled, targets to indenting and switching to debug/release mode are configured.")
+set(WB_ENABLE_TESTS ON CACHE BOOL "If enabled, tests are configured.")
+set(WB_ENABLE_DOCUMENTATION ON CACHE BOOL "If enabled, building of the documentation is configured.")


There is already an option called WB_BUILD_DOCUMENTATION in doc/CMakeList.txt. I am fine with removing that in favor of WB_ENABLE_DOCUMENTATION, but that would need to be done.

the option I introduced also doesn't do anything. :-) removed.

MFraters · 2024-02-29T14:24:16Z

CMakeLists.txt

@@ -40,6 +40,12 @@ ENDIF()

 set (WORLD_BUILDER_SOURCE_DIR ${PROJECT_SOURCE_DIR})

+# define some config options:
+set(WB_ENABLE_APPS ON CACHE BOOL "If enabled, applications gwb-grid and gwb-dat are built.")


The apps are used in the tester, so if this is off and the testers are on, it needs to only run the tests which do not require either gwb-grid or gwb-dat. Can you add a github tester which turns off the apps, but leaves the tester on?

For the test, please put it in test.yml and set needs: [test_indentation,coverage]. If you want me to add the tester, let me know, I am happy to do it.

tjhei · 2024-03-02T01:25:39Z

Ok, this should be good to go. Would you mind adding the additional CI step, @MFraters ?

This PR adds a number of CMake options to rename the name of the gwb library and to disable certain targets/features. This change allows us to configure a debug and a release mode build of GWB when ASPECT is configured as DebugRelease mode.

MFraters

Sure, I will make a follow up pull request for that. Thanks for working on this!

tjhei mentioned this pull request Feb 28, 2024

correctly support external GWB in DebugRelease geodynamics/aspect#5592

Merged

MFraters reviewed Feb 29, 2024

View reviewed changes

MFraters mentioned this pull request Feb 29, 2024

Release 0.6 #670

Closed

11 tasks

MFraters approved these changes Feb 29, 2024

View reviewed changes

tjhei added 2 commits March 1, 2024 20:26

CMake changes for ASPECT debugrelease mode

179e5d3

This PR adds a number of CMake options to rename the name of the gwb library and to disable certain targets/features. This change allows us to configure a debug and a release mode build of GWB when ASPECT is configured as DebugRelease mode.

disable tests we can not run

d148e94

tjhei force-pushed the cmake-subproject branch from 57f075b to d148e94 Compare March 2, 2024 01:26

fix typo

51d965f

MFraters approved these changes Mar 2, 2024

View reviewed changes

MFraters merged commit 6131388 into GeodynamicWorldBuilder:main Mar 2, 2024
34 of 35 checks passed

tjhei deleted the cmake-subproject branch March 2, 2024 01:54

MFraters mentioned this pull request Mar 2, 2024

add WB_ENABLE_APP = OFF test. #693

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CMake changes for ASPECT DebugRelease mode #686

CMake changes for ASPECT DebugRelease mode #686

tjhei commented Feb 28, 2024 •

edited

Loading

github-actions bot commented Feb 28, 2024 •

edited

Loading

MFraters Feb 29, 2024

codecov bot commented Feb 29, 2024 •

edited

Loading

tjhei commented Feb 29, 2024

MFraters left a comment

MFraters Feb 29, 2024

tjhei Feb 29, 2024

MFraters Feb 29, 2024

MFraters Feb 29, 2024

tjhei commented Mar 2, 2024

MFraters left a comment

CMake changes for ASPECT DebugRelease mode #686

CMake changes for ASPECT DebugRelease mode #686

Conversation

tjhei commented Feb 28, 2024 • edited Loading

github-actions bot commented Feb 28, 2024 • edited Loading

MFraters Feb 29, 2024

Choose a reason for hiding this comment

codecov bot commented Feb 29, 2024 • edited Loading

Codecov Report

tjhei commented Feb 29, 2024

MFraters left a comment

Choose a reason for hiding this comment

MFraters Feb 29, 2024

Choose a reason for hiding this comment

tjhei Feb 29, 2024

Choose a reason for hiding this comment

MFraters Feb 29, 2024

Choose a reason for hiding this comment

MFraters Feb 29, 2024

Choose a reason for hiding this comment

tjhei commented Mar 2, 2024

MFraters left a comment

Choose a reason for hiding this comment

tjhei commented Feb 28, 2024 •

edited

Loading

github-actions bot commented Feb 28, 2024 •

edited

Loading

codecov bot commented Feb 29, 2024 •

edited

Loading