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

Add h5 to testrunner and use static libraries in debug mode #172

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

jbreue16
Copy link
Contributor

To expand our tests (as proposed in #167 ), we want to use the standard CADET setup files in .h5 format for our reference solutions.
To read from h5 files in our tests, we include the h5 lib in the testrunner target.
We additionally link to static libraries to ease the use of debug builds.

@jbreue16 jbreue16 changed the title add h5 to testrunner and use static libraries in debug mode Add h5 to testrunner and use static libraries in debug mode Feb 26, 2024
@ronald-jaepel
Copy link
Collaborator

Alright, with this Pipeline we at least know, that the problem isn't in the CmakeSettings.json.
Apparently adding list(APPEND TEST_HDF5_TARGETS testRunner) alone breaks the CI. I don't have time today but hope I can look into this on Wednesday.

@ronald-jaepel ronald-jaepel force-pushed the feature/test_runner_hdf5 branch from dda9c49 to 191b402 Compare February 29, 2024 13:07
@jbreue16
Copy link
Contributor Author

@ronald-jaepel Do we want to keep the static linking in debug mode?

@ronald-jaepel
Copy link
Collaborator

ronald-jaepel commented Feb 29, 2024

Here's a blog post about static vs dynamic. It contains everything I know about the subject. :D AFAIK the main drawback of static links is:

  1. we potentially use more disk space
  2. if we want to update a dependency we need to rebuild CADET

I think both of those are non-issues for us, so I'd vote for keeping it statically linked. It solves all of those "Can't find x.dll" errors.

Edit: found another drawback:

Static linking is generally slower at build time, which can be annoying in an edit-build-debug cycle. Mergeable libraries behave like dynamic libraries in debug mode.

@jbreue16 jbreue16 requested a review from schmoelder February 29, 2024 16:05
@jbreue16
Copy link
Contributor Author

I agree to trade this potential drawback at buildtime for resolving the "Can't find x.dll" issues.
Ill ask @schmoelder for review, maybe he has the same opinion on this?

@schmoelder
Copy link
Contributor

schmoelder commented Feb 29, 2024

I don't know enough about dynamically/statically linked libraries, and even less so for Windows, so take my opinion with a grain of salt.

  • we potentially use more disk space

I don't see any issue here

  • if we want to update a dependency we need to rebuild CADET

So be it. We're just talking about debug mode here, right?

@ronald-jaepel
Copy link
Collaborator

Yes, just the debug mode. The release build is statically linked anyways and has afaik always been.

@jbreue16
Copy link
Contributor Author

Alright, lets merge this then.
This PR is a prerequisite for #167 :)

@ronald-jaepel ronald-jaepel merged commit 68c1f2a into master Feb 29, 2024
3 checks passed
@ronald-jaepel ronald-jaepel deleted the feature/test_runner_hdf5 branch February 29, 2024 17:30
@jbreue16 jbreue16 mentioned this pull request Mar 5, 2024
22 tasks
@jbreue16 jbreue16 mentioned this pull request Jul 26, 2024
42 tasks
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