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

Enable system installation of OpenRAND #22

Merged
merged 8 commits into from
Mar 11, 2024

Conversation

Shihab-Shahriar
Copy link
Collaborator

@Shihab-Shahriar Shihab-Shahriar commented Feb 13, 2024

Also, renames DEVICE macro to avoid potential conflict

@Shihab-Shahriar Shihab-Shahriar changed the title Renamed DEVICE macro to OPENRAND_DEVICE to avoid potential conflicts … Enable system installation of OpenRAND Feb 14, 2024
GoogleTest and GoogleBenchmark are no longer installed alongside OpenRAND.
Tests, benchmarks, and examples can be optionally enabled/disabled and they directly link against the OpenRAND interface lib.
Compiler flags are directly applied to the OpenRAND interface lib with considerations for different compilers.
OpenMP is now an optional dependency.
@palmerb4
Copy link
Collaborator

There are a couple issues/questions that need addressed within our CMakeLists.txt

  1. Google test and benchmark are currently installed alongside OpenRAND. We only need them for testing/benchmarking, so they shouldn't be within our install interface.
  2. Compilation of our tests is currently contingent on if CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR. This handles the case where OpenRAND is pulled in by an external library via fetch content but it does not handle optional enabling/disabling of tests/benchmarks/examples when compiling/installing OpenRAND directly.
  3. We hard-code our CMAKE_CXX_FLAGS in ways that are non-generalizable. For example, why should we enable -fopenmp if its a requirement of our tests/benchmarks/examples but not OpenRAND itself? As well, these flags are not cross compatible between different compilers, such as -Wdouble-promotion not working for Clang.
  4. Where do probdist mylib come from in target_link_libraries for TestU01?

I forked your pull request and modified OpenRAND to address 1-3. I issued a pull request to the cmake_update branch of Shihab-Shahriar/OpenRAND. If you merge, the modifications should automatically become a part of this pull request. Before you do, double check that the CI is fine. I made enabling tests, benchmarks, and examples optional, so if you want your CI to run the tests, you'd need to update the cmake build flags. I believe that will effect the CMake Configure of run-tests.yml, but I'm not familiar with github actions.

@Shihab-Shahriar
Copy link
Collaborator Author

@palmerb4 , I turned on the CI tests and updated doc. Re your q4, I largely followed this link for installing TestU01 (author of PCG).

@Shihab-Shahriar Shihab-Shahriar merged commit c995ff7 into msu-sparta:main Mar 11, 2024
2 checks passed
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.

2 participants