Build with pinned llvm-openmp

[xhochy]

This will allow people to use scikit-learn with llvm-openmp versions as old as the compiler. This is useful in other conda-builds where llvm-openmp is pinned to the clang version.

I did rerender locally, but no changes were produced.

[conda-forge-webservices]

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.

[jjerphan]

The same specifications are present in the build section.

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

[jjerphan]

@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.

[xhochy]

Yes! In the perfect world, they would suffice.

[jakirkham]

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?

[xhochy]

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.

[jakirkham]

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

[jeremiedbb]

in #241, we dropped these specifications in host because it should happen automatically. Has anything changed since then ?

[xhochy]

At least the outcome is that scikit-learn has a llvm-openmp>=1.8.5 dependency at the moment.

[xhochy]

Something with the run_exports seems to be not working. On the main build, we have 16.0.6 in build and implicitly(!) 18.1.5 in host. The run_exports are applied with the ones from host though.

[xhochy]

Running conda-render on main shows that we add the run_exports of both llvm-openmp instances:

  run:
    - llvm-openmp >=16.0.6
    - llvm-openmp >=18.1.5
 …

[xhochy]

It is either a openmp-feedstock or conda-build bug: conda-forge/openmp-feedstock#126 I have added a comment to the duplication in host so that we know why we currently have it.

[xhochy]

I did remember this is not the first time I saw this issue on this feedstock: #177

[xhochy]

Would you be OK with merging this? I hope I have sufficiently documented the reasoning/bugs.

[jjerphan]

LGTM. Thank you, @xhochy.

This documented workaround is sufficient to me.

I do not think we need to rebuild the previous versions.

I would wait for someone else's opinion before merging.

[xhochy]

I do not think we need to rebuild the previous versions.

No, they are mostly open as the change was only recently reverted.

FYI @jakirkham

[jakirkham]

Do we need to do something like this (though maybe with tweaks) to better explicitly align versions between build & host?

Suggested change

	# See https://github.com/conda-forge/openmp-feedstock/issues/126
	- llvm-openmp # [osx]
	- libgomp # [linux]
	# See https://github.com/conda-forge/openmp-feedstock/issues/126
	- llvm-openmp {{ llvm_openmp }} # [osx]
	- libgomp {{ libgomp }} # [linux]

Edit: This may need to be done with the compiler versions themselves

Suggested change

	# See https://github.com/conda-forge/openmp-feedstock/issues/126
	- llvm-openmp # [osx]
	- libgomp # [linux]
	# See https://github.com/conda-forge/openmp-feedstock/issues/126
	- llvm-openmp {{ c_compiler_version }} # [osx]
	- libgomp {{ c_compiler_version }} # [linux]

[xhochy]

No, they are already pinned in .ci_support:

scikit-learn-feedstock/.ci_support/osx_64_numpy2.0python3.11.____cpython.yaml

Lines 21 to 22 in 8bb0be3

	llvm_openmp:
	- '16'

[jakirkham]

Is that sufficiently pinned for our needs?

[xhochy]

Yes!

[jakirkham]

Sorry. Asking because this is only the major version and wasn't sure if that is enough for the use case that you asked about. If it is, great!

[jakirkham]

Thanks Uwe and Julien! 🙏