Skip to content

Conversation

@julienfausty
Copy link

@julienfausty julienfausty commented Jan 24, 2023

This pull request comes in support of this one in GEOSX: GEOS-DEV/GEOS#2250

It performs the following operations
Add new dependencies into thirdPartyLibs repository:

  • catalyst itself version 2.0.0-rc3
  • adios/catalyst implementation on commit af3b35f
  • adios2 v2.8.3
  • update conduit from 0.8.2 to 0.8.6
  • update VTK to 49361a2 for mangling capabilities

################################
set(CONDUIT_DIR "${CMAKE_INSTALL_PREFIX}/conduit")
set(CONDUIT_URL "${TPL_MIRROR_DIR}/conduit-0.8.2.tar.gz")
set(CONDUIT_URL "${TPL_MIRROR_DIR}/conduit-0.8.6.zip")

Choose a reason for hiding this comment

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

this is not the Catalyst version, is this wanted ?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I don't think we should tether geosx's conduitversion to catalyst's. This is just an update of the conduit package to get some things compiling

@julienfausty julienfausty force-pushed the add-catalyst-and-adios branch from b0a01ba to 7a00253 Compare March 6, 2023 08:44
@julienfausty julienfausty force-pushed the add-catalyst-and-adios branch 2 times, most recently from acf1e38 to 84f47f5 Compare March 14, 2023 08:31
@julienfausty julienfausty force-pushed the add-catalyst-and-adios branch 2 times, most recently from 0fdc45f to 47f28b0 Compare April 5, 2023 12:57
@julienfausty julienfausty force-pushed the add-catalyst-and-adios branch from 47f28b0 to a716a79 Compare April 26, 2023 07:34
@julienfausty julienfausty force-pushed the add-catalyst-and-adios branch from a716a79 to 4a96262 Compare July 20, 2023 07:52
Mangling VTK uses an inline namespace to change the VTK symbols in the
binary so as to allow multiple VTK binaries to coexist
@julienfausty julienfausty force-pushed the add-catalyst-and-adios branch from 4a96262 to ff57a33 Compare July 20, 2023 08:27
* downgrade fmt 10.0.0 -> 9.1.0
* patch raja for stdexcept and cstdlib
@julienfausty julienfausty force-pushed the add-catalyst-and-adios branch from 26b0f75 to 6e1dad0 Compare September 7, 2023 13:37
Copy link

@CharlesGueunet CharlesGueunet left a comment

Choose a reason for hiding this comment

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

LGTM after small questions / remarks

if( ENABLE_VTK )
set( VTK_DIR "${CMAKE_INSTALL_PREFIX}/vtk" )
set( VTK_URL "${TPL_MIRROR_DIR}/VTK-9.2.6.tar.gz" )
set( VTK_URL "${TPL_MIRROR_DIR}/VTK-59d89108f4.tar.gz" )

Choose a reason for hiding this comment

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

you do not need to change, but a think a nightly (with a date as a version) is slightly more readable.
Can you comment that this is between 9.2.6 and 9.3.0 ?

Copy link
Author

Choose a reason for hiding this comment

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

yep, I was not aware of the nightly VTK downloads, I will use that and update the repo

Copy link
Author

Choose a reason for hiding this comment

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

see below

CMakeLists.txt Outdated
################################
set(FMT_DIR "${CMAKE_INSTALL_PREFIX}/fmt")
set(FMT_URL "${TPL_MIRROR_DIR}/fmt-10.0.0.tar.gz")
set(FMT_URL "${TPL_MIRROR_DIR}/fmt-9.1.0.zip")

Choose a reason for hiding this comment

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

why a downgrade ?

Copy link
Author

Choose a reason for hiding this comment

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

was a compiler issue with gcc14

PATCH_COMMAND patch -p0 < ${TPL_MIRROR_DIR}/vtkXMLReader-fpe.patch
BUILD_COMMAND ${TPL_BUILD_COMMAND}
INSTALL_COMMAND "${TPL_INSTALL_COMMAND}"
PATCH_COMMAND patch -p1 < ${TPL_MIRROR_DIR}/VTK-ABI.patch

Choose a reason for hiding this comment

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

why is this a patch ? seems like it should be updated upstream

Copy link
Author

Choose a reason for hiding this comment

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

This has been updated upstream, but to avoid wasting time with the merge I made this patch and then forgot about it. I will update VTK with a nightly build and take out the patch

Copy link
Author

Choose a reason for hiding this comment

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

the 9.3.0-rc1 version has a broken ABI, let's wait for the release before making the changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants