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

[OpenBLAS/lapack] Apply the patch from xianyi/OpenBLAS#3392 #3708

Merged
merged 14 commits into from
Oct 8, 2021

Conversation

DilumAluthge
Copy link
Member

@DilumAluthge DilumAluthge commented Oct 7, 2021

This pull request applies the patch from OpenMathLib/OpenBLAS#3392 to the following JLLs:

  1. OpenBLAS_jll
  2. OpenBLAS32_jll
  3. OpenBLASHighCoreCount_jll

Refs JuliaLang/julia#42415
Refs Reference-LAPACK/lapack#625

@DilumAluthge
Copy link
Member Author

@Keno Which version of OpenBLAS should I be applying this patch to? It seems to be "failing to apply the patch" when I try to apply it to OpenBLAS 0.3.10.

@Keno
Copy link
Contributor

Keno commented Oct 7, 2021

OpenBLAS vendors lapack in one of it's source directories. You'll have to adjust the patch file or patch invocation accordingly.

@DilumAluthge DilumAluthge changed the title Apply the patch from Reference-LAPACK/lapack#625 (ref JuliaLang/julia#42415) Apply the patch from xianyi/OpenBLAS#3392 (ref JuliaLang/julia#42415, ref Reference-LAPACK/lapack#625) Oct 7, 2021
@DilumAluthge DilumAluthge changed the title Apply the patch from xianyi/OpenBLAS#3392 (ref JuliaLang/julia#42415, ref Reference-LAPACK/lapack#625) [OpenBLAS] Apply the patch from xianyi/OpenBLAS#3392 (ref JuliaLang/julia#42415, ref Reference-LAPACK/lapack#625) Oct 7, 2021
@DilumAluthge
Copy link
Member Author

OpenBLAS vendors lapack in one of it's source directories. You'll have to adjust the patch file or patch invocation accordingly.

Alright, I'm now using the patch file from the relevant PR to OpenBLAS (OpenMathLib/OpenBLAS#3392).

@DilumAluthge DilumAluthge added julia 💜 ❤️ 💚 Builders and issues related to Julia and its dependencies linear algebra 🔢 labels Oct 7, 2021
@DilumAluthge
Copy link
Member Author

DilumAluthge commented Oct 7, 2021

It looks like the patch is applying correctly now. I put it in the three folders corresponding to OpenBLAS version 0.3.10. Is that sufficient, or should I also put it into the folders for other versions of OpenBLAS?

@DilumAluthge DilumAluthge force-pushed the dpa/julia42415-lapack625 branch 2 times, most recently from 6513f27 to 4f7277c Compare October 7, 2021 22:25
@DilumAluthge DilumAluthge marked this pull request as draft October 7, 2021 23:31
@DilumAluthge
Copy link
Member Author

I guess I don't understand how the patch folders work here. If I add the patch file to OpenBLAS 0.3.10, does it automatically get applied to later OpenBLAS versions as well? Or do I manually need to symlink my patch file into the bundled/patches folder of each OpenBLAS version to which I want to apply the patch?

@staticfloat
Copy link
Member

Yes, it needs to be symlinked into every patches folder that needs it.

@DilumAluthge DilumAluthge removed the request for review from ViralBShah October 8, 2021 03:25
@DilumAluthge DilumAluthge force-pushed the dpa/julia42415-lapack625 branch 2 times, most recently from 5ea1875 to 41ec23f Compare October 8, 2021 04:01
@DilumAluthge DilumAluthge changed the title [OpenBLAS] Apply the patch from xianyi/OpenBLAS#3392 (ref JuliaLang/julia#42415, ref Reference-LAPACK/lapack#625) [OpenBLAS/lapack] Apply the patch from xianyi/OpenBLAS#3392 (ref JuliaLang/julia#42415, ref Reference-LAPACK/lapack#625) Oct 8, 2021
@DilumAluthge DilumAluthge changed the title [OpenBLAS/lapack] Apply the patch from xianyi/OpenBLAS#3392 (ref JuliaLang/julia#42415, ref Reference-LAPACK/lapack#625) [OpenBLAS/lapack] Apply the patch from xianyi/OpenBLAS#3392 Oct 8, 2021
@DilumAluthge
Copy link
Member Author

The slicing expansion resulted in 257 jobs which exceeds the maximum allowable job count of 256.

😭 😭 😭

@vchuravy
Copy link
Member

vchuravy commented Oct 8, 2021

Do the HighBlasCount ones in a second PR

@ViralBShah
Copy link
Member

Most likely, we won't need the highcorecounts going forward now that #3667 is merged.

@vchuravy vchuravy marked this pull request as ready for review October 8, 2021 19:20
@vchuravy vchuravy enabled auto-merge (squash) October 8, 2021 21:03
@DilumAluthge
Copy link
Member Author

@vchuravy I disabled auto-merge because they are two CI failures.

@vchuravy vchuravy merged commit df015c7 into master Oct 8, 2021
@vchuravy vchuravy deleted the dpa/julia42415-lapack625 branch October 8, 2021 21:22
@ViralBShah
Copy link
Member

Have we applied this patch to the julia source build too? I believe the next step might be to bump this for master and if everything looks ok, we do the same for 1.6 and 1.7.

simeonschaub pushed a commit to simeonschaub/Yggdrasil that referenced this pull request Feb 23, 2022
…iaPackaging#3708)

* [OpenBLAS/lapack] Apply the patch from OpenMathLib/OpenBLAS#3392

Co-authored-by: Valentin Churavy <v.churavy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
julia 💜 ❤️ 💚 Builders and issues related to Julia and its dependencies linear algebra 🔢
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants