Skip to content

fix OpenBLAS 0.3.15 patch to correctly set the CPU core type for Tiger Lake#15845

Merged
akesandgren merged 3 commits intoeasybuilders:developfrom
micaeljtoliveira:20220713201058_new_pr_OpenBLAS0315
Jan 11, 2023
Merged

fix OpenBLAS 0.3.15 patch to correctly set the CPU core type for Tiger Lake#15845
akesandgren merged 3 commits intoeasybuilders:developfrom
micaeljtoliveira:20220713201058_new_pr_OpenBLAS0315

Conversation

@micaeljtoliveira
Copy link
Copy Markdown
Contributor

(created using eb --new-pr)

@boegel boegel added the bug fix label Aug 3, 2022
@boegel boegel added this to the next release (4.6.1?) milestone Aug 3, 2022
@boegel boegel changed the title Fix OpenBLAS 0.3.15 patch to correctly set the CPU core type for Tiger Lake. fix OpenBLAS 0.3.15 patch to correctly set the CPU core type for Tiger Lake Aug 3, 2022
akesandgren
akesandgren previously approved these changes Aug 31, 2022
Copy link
Copy Markdown
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, patch matches changes from 0.3.15 to 0.3.18

@akesandgren
Copy link
Copy Markdown
Contributor

@boegelbot Please test @ generoso

@boegelbot
Copy link
Copy Markdown
Collaborator

@akesandgren: Request for testing this PR well received on login1

PR test command 'EB_PR=15845 EB_ARGS= /opt/software/slurm/bin/sbatch --job-name test_PR_15845 --ntasks=4 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

  • exit code: 0
  • output:
Submitted batch job 9092

Test results coming soon (I hope)...

Details

- notification for comment with ID 1232778622 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

@boegelbot
Copy link
Copy Markdown
Collaborator

Test report by @boegelbot
FAILED
Build succeeded for 1 out of 2 (2 easyconfigs in total)
cns2 - Linux Rocky Linux 8.5, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/89d26aff7ca2dcc5596ba64568a108c0 for a full test report.

@akesandgren
Copy link
Copy Markdown
Contributor

The updated patch fails to apply to 0.3.12.
Please provide a working, separate, one for 0.3.12

@micaeljtoliveira
Copy link
Copy Markdown
Contributor Author

@akesandgren Sorry for the long delay in addressing the problem with 0.3.12. I tried to fix the patch, but it seems that for it to actually work one would need to backport a lot more changes from later versions of OpenBLAS. So instead I just kept the current patch as it is for that version. Hope that's okay.

@akesandgren
Copy link
Copy Markdown
Contributor

Test report by @akesandgren
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in total)
b-an02.hpc2n.umu.se - Linux Ubuntu 20.04, x86_64, Intel(R) Xeon(R) CPU E5-2690 v4 @ 2.60GHz, Python 3.8.10
See https://gist.github.com/482c201aa6e3227e04338df8ec8a4061 for a full test report.

@akesandgren
Copy link
Copy Markdown
Contributor

@micaeljtoliveira Can you take a look at this, https://gist.github.com/sassy-crick/cc750a24bd1404c1c9a6f8a2c09c10aa ?

Built on a "11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz"

@easybuilders easybuilders deleted a comment from boegelbot Jan 3, 2023
@micaeljtoliveira
Copy link
Copy Markdown
Contributor Author

@akesandgren Not quite sure what to do about this. The issue with 0.3.12 is to be expected. As mentioned above, to fix it one would need to backport more fixes made in later versions of OpenBLAS. I tried to do that, but without much success: compilation what still broken even after including several more changes to the patch. Regarding the Lapack timing, that's a strange error, as I can't see any changes in the PR that could do that.

@akesandgren
Copy link
Copy Markdown
Contributor

Just wanted you to verify that it's what you'd expect for the 0.3.12 build.
The timing error can be ignored, we don't use that part as far as I know.

Copy link
Copy Markdown
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@akesandgren
Copy link
Copy Markdown
Contributor

Going in, thanks @micaeljtoliveira!

@akesandgren akesandgren merged commit 07b2b0c into easybuilders:develop Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants