Skip to content

Build with pinned llvm-openmp #265

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

Merged
merged 2 commits into from
May 23, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion recipe/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ source:
sha256: cd2aac2b566c7e740d34aabb4737864c74ba33c29aad7101fbf1bab2931c02dc

build:
number: 0
number: 1
skip: true # [py<39]

requirements:
Expand Down Expand Up @@ -40,6 +40,8 @@ requirements:
- scipy >=1.8 # [aarch64]
- joblib >=1.2.0
- threadpoolctl >=3.1.0
- llvm-openmp # [osx]
- libgomp # [linux]
Comment on lines +44 to +45
Copy link
Member

Choose a reason for hiding this comment

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

The same specifications are present in the build section.

Should it be removed for the current on that you propose?

Copy link
Member

@jjerphan jjerphan May 23, 2024

Choose a reason for hiding this comment

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

@xhochy: why mustn't the specification line 28 and line 29 be removed?

Edit: they must be present in this section, see the reasons in the documentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes! In the perfect world, they would suffice.

Copy link
Member

@jakirkham jakirkham May 23, 2024

Choose a reason for hiding this comment

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

Could we just add the desired pin in build?

Should add I'm confused as to why the requirement in build is not already aligned with the compiler, which it is installed next to

Edit: Maybe an example of the issue encountered would help. Is there one somewhere we can look at?

Copy link
Member Author

Choose a reason for hiding this comment

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

I outlined the issue in conda-forge/openmp-feedstock#126 We actually install the desired pin in build but due to conda-builds internal behaviour, the run_exports is still taken from host. Thus, I don't see a solution where we don't add anything to host.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok. Thank you Uwe! Very helpful 🙂

Wrote up a reply there on what we could consider next: conda-forge/openmp-feedstock#126 (comment)

Agree this is the best we can do for now

run:
- python
- scipy
Expand Down