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

Build for 3.10.1 #51

Merged
merged 12 commits into from
Apr 24, 2023
Merged

Build for 3.10.1 #51

merged 12 commits into from
Apr 24, 2023

Conversation

h-vetinari
Copy link
Member

@h-vetinari h-vetinari commented Mar 15, 2021

Upstream unexpectedly released 3.9.1 (unexpected because 3.10 was planned for end of February), let's try building it.

Edit: actually, the tag was removed again and replaced with 3.9.1-pre.
Edit 2: final 3.9.1 tag has been republished

Closes #54
Closes #58
Closes #59

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • It looks like the 'libblas' output doesn't have any tests.
  • It looks like the 'libcblas' output doesn't have any tests.
  • It looks like the 'liblapack' output doesn't have any tests.
  • It looks like the 'liblapacke' output doesn't have any tests.
  • It looks like the 'blas-devel' output doesn't have any tests.
  • It looks like the 'lapack' output doesn't have any tests.

@h-vetinari h-vetinari changed the title Build for 3.9.1 Build for 3.9.1-pre Mar 28, 2021
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • It looks like the 'libblas' output doesn't have any tests.
  • It looks like the 'libtmglib' output doesn't have any tests.
  • It looks like the 'libcblas' output doesn't have any tests.
  • It looks like the 'liblapack' output doesn't have any tests.
  • It looks like the 'liblapacke' output doesn't have any tests.
  • It looks like the 'blas-devel' output doesn't have any tests.
  • It looks like the 'lapack' output doesn't have any tests.

@h-vetinari h-vetinari changed the title Build for 3.9.1-pre Build for 3.9.1 May 8, 2021
@h-vetinari
Copy link
Member Author

Opened an issue upstream for the one remaining windows failure: Reference-LAPACK/lapack#548

@h-vetinari h-vetinari marked this pull request as draft May 14, 2021 12:32
@h-vetinari
Copy link
Member Author

3835a4d was an upstream ask for debugging; let's not merge this as is just yet - ideally we can figure out Reference-LAPACK/lapack#548 and backport the patch.

@h-vetinari h-vetinari marked this pull request as ready for review May 17, 2021 20:46
@h-vetinari
Copy link
Member Author

Upstream issue has been fixed, patch backport has been cleaned up, and this is ready for review @conda-forge/lapack.

I dared to add myself to the maintainers, since I'm now already involved in most of the CF blas-stack, but happy to take out that last commit if desired.

@h-vetinari
Copy link
Member Author

Drone just timed out (and is not susceptible to close/open) 😒

@h-vetinari
Copy link
Member Author

drone being difficult...

fatal: unable to access 'https://github.com/conda-forge/lapack-feedstock.git/': Failed to connect to github.com port 443: Operation timed out

@h-vetinari
Copy link
Member Author

@isuruf
What do you think about lapack 3.9.1 - this would be ready? Should mainly be bugfixes, all the real features (AFAICT) were merged only for the upcoming 3.10.0.

@isuruf
Copy link
Member

isuruf commented May 21, 2021

Does openblas, mkl suport lapack 3.9.1? If not, we can't merge this.

@martin-frbg
Copy link

Regarding openblas, the sudden release of 3.10.0 took me by surprise - while I have kept cherry-picking from their development branch, I would still need to integrate at least their patches for CBLAS and LAPACKE compatibility with Fortran "hidden" character length arguments. (And there is a ton of non-code differences not yet merged: some improved comments on the algorithms used in a few files, and above all the removal of all dates and version numbers from all of their source files)

@h-vetinari
Copy link
Member Author

Thanks @martin-frbg - 3.10.0 will take a while I guess (so no rush for openblas) - however, this PR is just about 3.9.1. 🙃

@h-vetinari
Copy link
Member Author

@conda-forge-admin, please rerender

@h-vetinari
Copy link
Member Author

@isuruf, I'm trying to backport Reference-LAPACK/lapack#597, but it does not apply easily to 3.9.0. I can try to hack something together, but I'd really rather merge this and just delay the MKL 3.9.1 builds (as you did for 3.9.0).

@martin-frbg
Copy link

IIRC, 3.9.0 did not take any special measures to ensure some tests are run single-threaded only. So the code you want to patch may not actually be there yet.

@h-vetinari
Copy link
Member Author

IIRC, 3.9.0 did not take any special measures to ensure some tests are run single-threaded only. So the code you want to patch may not actually be there yet.

The issue that Isuru is concerned about - to my understanding - is less about the single threading and more the compatibility with lapack 3.9.1 across the different versions that conda-forge supports. Openblas 0.3.17 obviously does support 3.9.1 already, but MKL doesn't yet.

For 3.9.0, netlib/openblas were published before MKL (from the POV of conda-forge), but since I haven't heard back I'm not sure what other concerns Isuru might have.

@h-vetinari
Copy link
Member Author

Seems this will take a while longer still... MKL decided to skip 3.9.1. and go for 3.10.0 directly.

I still don't understand why we cannot proceed as with 3.9-vs.-3.8, but at this point, I've given up in trying to get an answer on this.

@h-vetinari h-vetinari changed the title Build for 3.9.1 Build for 3.10.1 Apr 23, 2023
@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • It looks like the 'libblas' output doesn't have any tests.
  • It looks like the 'libtmglib' output doesn't have any tests.
  • It looks like the 'libcblas' output doesn't have any tests.
  • It looks like the 'liblapack' output doesn't have any tests.
  • It looks like the 'liblapacke' output doesn't have any tests.
  • It looks like the 'blas-devel' output doesn't have any tests.
  • It looks like the 'lapack' output doesn't have any tests.

@h-vetinari
Copy link
Member Author

Took me a while to pick up this PR after it sat in an echo chamber for so long, but it's green on first push! 🙃

In any case, MKL supports LAPACK 3.10.1 since 2022.2, so does openblas since 0.3.21, and blis thinks they're compatible too, or will at least endeavour to fix issues as they arise.

PTAL @conda-forge/lapack

@isuruf
Copy link
Member

isuruf commented Apr 24, 2023

Have you checked that conda-forge/blas-feedstock#96 works by building this locally?

@h-vetinari
Copy link
Member Author

Have you checked that conda-forge/blas-feedstock#96 works by building this locally?

Given that all implementations claim compatibility, I'm planning to figure this out on the blas feedstock (and would follow up with the respective implementations if they're failing tests or whatever). I don't have the hardware to test this across platforms in advance, and I don't think that's a reasonable bar in this situation?

@isuruf
Copy link
Member

isuruf commented Apr 24, 2023

Once we merge this PR, conda will try to install libblas=3.10.1 because that's the latest version and then users will get the netlib variant which is the only implementation available.

@h-vetinari
Copy link
Member Author

I understand that. Alternatives are pushing this into a dev label (that we can pull in on the blas PR), or to revive conda-forge/blas-feedstock#68 (build all variants including netlib on one feedstock)

@isuruf
Copy link
Member

isuruf commented Apr 24, 2023

Let's push to a rc label

@h-vetinari
Copy link
Member Author

Let's push to a rc label

Done 👍

@isuruf
Copy link
Member

isuruf commented Apr 24, 2023

Thanks

@isuruf isuruf added the automerge Merge the PR when CI passes label Apr 24, 2023
@github-actions github-actions bot merged commit f675059 into conda-forge:main Apr 24, 2023
@github-actions
Copy link
Contributor

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

  • linter: passed
  • travis: passed
  • azure: passed

Thus the PR was passing and merged! Have a great day!

@h-vetinari h-vetinari deleted the bump branch April 24, 2023 07:47
@h-vetinari h-vetinari mentioned this pull request Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge the PR when CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants