-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
cmaes: Add 0.10.0 #24202
base: master
Are you sure you want to change the base?
cmaes: Add 0.10.0 #24202
Conversation
* Protobug mutator compatible with Conan 2.x Signed-off-by: Uilian Ries <[email protected]> * Add version 1.3 Signed-off-by: Uilian Ries <[email protected]> * Enforce protobuf link Signed-off-by: Uilian Ries <[email protected]> * update abseil usage Signed-off-by: Uilian Ries <[email protected]> * consume all libs in tests Signed-off-by: Uilian Ries <[email protected]> * define cxx language in cmakelists Signed-off-by: Uilian Ries <[email protected]> * static-library on Windows Signed-off-by: Uilian Ries <[email protected]> * Exclude examples from CMakeLists.txt The upstream uses EXCLUDE_FROM_ALL CMake feature to avoid building examples by default, however, CMake still parses the example cmake file and requires more external dependencies like expat. Signed-off-by: Uilian Ries <[email protected]> * Do not build shared on Windows Signed-off-by: Uilian Ries <[email protected]> --------- Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@phbasler Thank you for your PR! Please, take a look in you emails/accounts used in this PR: #24202 (comment) The CLA bot listed 2 emails and it will not approve if both agrees to the CLA. |
@uilianries |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Conan v1 pipeline ✔️All green in build 29 (
Conan v2 pipeline ✔️
All green in build 29 (
|
@AbrilRBS why did you remove the openmp option? Should the package not be used anymore? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Thanks for taking the time to create the PR, sorry that we didn't get back to you on a timely manner. I've left some comments on the PR, and made a cleanup now that Conan 1 support is no longer needed :)
diff --git a/CMakeLists.txt b/CMakeLists.txt | ||
index 4a38212..dccfea4 100644 | ||
--- a/CMakeLists.txt | ||
+++ b/CMakeLists.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this comes from CMA-ES/libcmaes#234, do you have any insight into wether it will be merged eventually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope so. I will trigger @beniz again. If there is a new version with the PR, then I can adjust the conanfile here to not include this patch for the new version
mutable std::normal_distribution<Scalar> norm; // gaussian combinator | ||
|
||
- EIGEN_EMPTY_STRUCT_CTOR(scalar_normal_dist_op) | ||
+ scalar_normal_dist_op() = default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see this change referenced anywhere in the logs/upstream. Is this an issue on the library itself? If this is a backport, it'd be great to separate it to its own patch, so that we can crossreference back to it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is removed in the current eigen master branch https://gitlab.com/libeigen/eigen/-/merge_requests/946
I also got some problems because this additonally defines an empty copy ctr and we needed it to copy the members
|
||
int main() | ||
{ | ||
test1(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could probably simplify this test_package, the less actual computation, the better and the more probable it is for the test package to stay valid for future versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could remove the first test and just use the second test with the commented lines removed. Would that be ok?
Not that is should not be used per-se, it's more that its funcionality currently does not align with our expectations on CCI |
Specify library name and version: cmaes/0.10.0
Add new package cmaes according to issue #24201
Fixes #24201
I am not the author of cmaes, but according to the authors in CMA-ES/libcmaes#233 I can provide this recipe.