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

Bump minimum Armadillo version to 10.8 #404

Merged
merged 18 commits into from
Jul 14, 2024
Merged

Bump minimum Armadillo version to 10.8 #404

merged 18 commits into from
Jul 14, 2024

Conversation

conradsnicta
Copy link
Contributor

@conradsnicta conradsnicta commented Jul 9, 2024

Bump minimum Armadillo version to 10.8.2.

This allows using the numerous Armadillo API additions and enhancements since 9.800. It also allows assuming throughout the ensmallen codebase that matrices are initialised to zero by default.

Current state of Armadillo versions in various distros:

  • Ubuntu 22.04 LTS has Armadillo 10.8.2
  • Ubuntu 24.04 LTS has Armadillo 12.6.7
  • Debian 12 has Armadillo 11.4.2
  • Debian 13 (testing) has Armadillo 12.8.2
  • RHEL EPEL 8 & 9 has Armadillo 12.6.6 (subject to future upgrades)

Related: mlpack/mlpack#3760

@zoq
Copy link
Member

zoq commented Jul 10, 2024

Looks like the docker images to build uses an older version, will update it tomorrow.

@conradsnicta
Copy link
Contributor Author

@zoq Sounds good.

Copy link
Member

@shrit shrit left a 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, I will approve it after the CI has passed

@rcurtin
Copy link
Member

rcurtin commented Jul 12, 2024

Thanks for all the ifdef cleanups---I didn't realize there was so much of that going on in ensmallen. I'll approve too once we get CI passing and then we can get this merged and a new release out. 👍

@conradsnicta
Copy link
Contributor Author

@shrit @rcurtin @zoq I've updated the CI config via commits 32015ae and 51c9506
Let me know if this isn't the right way or something else needs to be done

@zoq
Copy link
Member

zoq commented Jul 12, 2024

Perfect, I just need to rebuild the docker image that we use on the jenkins server, and that should fix the ensmallen sanity checks job as well.

@conradsnicta
Copy link
Contributor Author

@zoq Do you mean updating the Ubuntu image? I think the current image uses the old Ubuntu 20.04 release which only has armadillo 9.800.4

@zoq
Copy link
Member

zoq commented Jul 12, 2024

@zoq Do you mean updating the Ubuntu image? I think the current image uses the old Ubuntu 20.04 release which only has armadillo 9.800.4

Right, updated the image.

@zoq
Copy link
Member

zoq commented Jul 12, 2024

@mlpack-jenkins test this please

2 similar comments
@zoq
Copy link
Member

zoq commented Jul 12, 2024

@mlpack-jenkins test this please

@zoq
Copy link
Member

zoq commented Jul 12, 2024

@mlpack-jenkins test this please

@conradsnicta
Copy link
Contributor Author

@rcurtin @shrit Could you provide the second approval to facilitate merging? The mlpack bot (that automatically provides the second approval after 24 hours) seems to have gone missing in action.

@conradsnicta conradsnicta requested a review from shrit July 14, 2024 12:53
@rcurtin
Copy link
Member

rcurtin commented Jul 14, 2024

@conradsnicta oh, you're right, I need to add the Github actions script to this repo too. (Basically I went to update mlpack-bot because it stopped working, then it turned out everything it was built on was deprecated, and so in the end I rewrote its functionality as a Github action.)

@rcurtin rcurtin merged commit 6b0a003 into master Jul 14, 2024
5 checks passed
@rcurtin rcurtin deleted the arma-version-bump branch July 14, 2024 20:18
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.

4 participants