-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Bump minimum Armadillo version to 10.8 #3760
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
Conversation
|
Looks good; we can also remove these legacy workarounds now: $ grep -r ARMA_VERSION .
./mlpack/core/util/arma_traits.hpp:#if ((ARMA_VERSION_MAJOR >= 10) || \
./mlpack/core/util/arma_traits.hpp: ((ARMA_VERSION_MAJOR == 9) && (ARMA_VERSION_MINOR >= 869)))
./mlpack/bindings/go/mlpack/capi/arma_util.hpp: #if ARMA_VERSION_MAJOR >= 10
./mlpack/bindings/python/mlpack/arma_util.hpp: #if ARMA_VERSION_MAJOR >= 10
./mlpack/bindings/julia/julia_util.cpp: #if ARMA_VERSION_MAJOR >= 10
./mlpack/bindings/julia/julia_util.cpp: #if ARMA_VERSION_MAJOR >= 10
./mlpack/bindings/julia/julia_util.cpp: #if ARMA_VERSION_MAJOR >= 10
./mlpack/bindings/julia/julia_util.cpp: #if ARMA_VERSION_MAJOR >= 10
./mlpack/bindings/julia/julia_util.cpp: #if ARMA_VERSION_MAJOR >= 10
./mlpack/bindings/julia/julia_util.cpp: #if ARMA_VERSION_MAJOR >= 10
./mlpack/bindings/julia/julia_util.cpp: #if ARMA_VERSION_MAJOR >= 10I think we will have to wait on the ensmallen version to get bumped but that shouldn't take long. |
|
@rcurtin legacy workarounds removed |
shrit
left a comment
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.
Looks good to me
shrit
left a comment
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.
Requesting change so we fix the CI before
rcurtin
left a comment
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.
Thanks, everything looks great. I pushed some simple changes to update the version used in CI; I'll work to get everything green there.
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.
Second approval provided automatically after 24 hours. 👍
|
@mlpack-jenkins test this please |
Bump minimum Armadillo version to 10.8.
This allows using the numerous Armadillo API additions and enhancements since 9.800. It also allows assuming throughout the mlpack codebase that matrices are initialised to zero by default.
Current state of Armadillo versions in various distros:
Related: mlpack/ensmallen#404