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 9.800. #318

Merged
merged 4 commits into from Aug 30, 2021

Conversation

zoq
Copy link
Member

@zoq zoq commented Aug 25, 2021

As discussed here #315 we like to bump the minimum Armadillo version to 9.800. I think once merged we should do another release 2.18.0.

.travis.yml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@conradsnicta
Copy link
Contributor

The index.md on the ensmallen website would also need to be changed to reflect the updated version requirement:
https://github.com/mlpack/ensmallen.org/blob/master/index.md

Copy link

@mlpack-bot mlpack-bot bot left a 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. 👍

@rcurtin
Copy link
Member

rcurtin commented Aug 28, 2021

When this is merged, we should also bump it in mlpack, otherwise we can get a situation where mlpack's Armadillo version requirement is less than ensmallen's, so a user could feasible download mlpack, download Armadillo 8.400.0, think "I'm good to go!", then suddenly find out that the newest ensmallen (via the autodownloader) actually needs an even newer version.

@shrit
Copy link
Member

shrit commented Aug 28, 2021

It is strange the error says it is unable to find Armadillo on windows
As @rcurtin said, we need to update it on the mlpack side as well. Maybe do that before merging this one?

@zoq
Copy link
Member Author

zoq commented Aug 29, 2021

I'll fix the windows build.

@zoq
Copy link
Member Author

zoq commented Aug 29, 2021

The auto-downloader downloads the ensmallen master branch? If that is the case, we should change that behavior, it should always download the latest stable version. Anyway, I'll open another PR on mlpack. With the release of another ensmallen, I also open a second PR to update ensmallen.org.

@rcurtin
Copy link
Member

rcurtin commented Aug 30, 2021

The auto-downloader downloads the ensmallen master branch? If that is the case, we should change that behavior, it should always download the latest stable version.

Right, it just downloads the latest stable version. So, we should just merge mlpack/mlpack#3043 before releasing another version of ensmallen. 👍

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.

None yet

4 participants