search

imcs-compsim / MIRCO

A shared-memory parallel BEM code for the contact of rough surfaces
MIT License
3 stars 3 forks source link

Add Cmake variables for googletest and Trilinos #75

Closed RShaw026 closed 1 year ago

RShaw026 commented 1 year ago

Description and Context

This will enable MIRCO to be used as a library in projects where the dependencies of MIRCO, i.e. googletest and Trilinos, are already present so that the problems of duplicate dependencies can be avoided.

Related Issues and Pull Requests

How Has This Been Tested?

code builds, and tests are passing. Tested with BACI, using FetchContent and commit hash.

Checklist

Additional Information

Interested Parties / Possible Reviewers

@mayrmt

mayrmt commented 1 year ago

In principle, this looks good. Do we have specific requirements on the version of Trilinos and googletest? If yes, then we should make sure, that they are met.

For Trilinos, I believe that we currently do not rely on a specific version. Since we just use Teuchos, which is well matured and stable, a version check does not bring any benefits. That might change in the future, if we use other actively developed packages, but for now there's seems to be no need for a version check.

@RShaw026 Do you have insight into googletest in that regard?

RShaw026 commented 1 year ago

@mayrmt I am not sure if I understand your question correctly. Once in a while, I update the googletest submodule in MIRCO and change the tests if required. Since we don't need to test MIRCO gtest when MIRCO is integrated into BACI, should I also not build the tests by moving that part of code inside the GTEST_IN_MIRCO flag? That way even if the googletest in BACI is incompatible with the tests written in MIRCO, it won't fail the BACI build in any way.

mayrmt commented 1 year ago

@RShaw026 I haven't thought about that, but you're right: moving the MIRCO tests inside GTEST_IN_MIRCO would be the consequence. How much effort is this? What would be the impact on further development and adding tests in MIRCO?

RShaw026 commented 1 year ago

@mayrmt The flag in the do-configure never needs to be changed. It will always be turned on. BACI can still fetch MIRCO with an OFF flag.

Strangely, if it is turned off in the do-configure file, the pipeline buildtest still passes (I checked it now). Should we need the fix that? https://github.com/imcs-compsim/MIRCO/actions/runs/4204799074 Is there a way to make the tests fail if the tests are not found?

mayrmt commented 1 year ago

@RShaw026 Well, if you guard the tests and disable the guarding flag, then they are not built, so they can't fail. That's the expected behavior. I think, we can leave it as is. If somebody disables the tests in the configure script, then we have to realize it and change it back -- but that's the same as someone wanted to comment out the tests.