Create post-link script to install Julia deps #41

MilesCranmer · 2022-08-14T05:14:13Z

Attempts to add a post-link script to install PySR's julia dependencies.

Checklist

Used a personal fork of the feedstock to propose changes
Bumped the build number (if the version is unchanged)
Reset the build number to 0 (if the version changed)
Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
Ensured the license file is being packaged.

Addresses #38.

conda-forge-linter · 2022-08-14T05:14:17Z

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.

ngam · 2022-08-14T13:43:01Z

Could you add this line to conda-forge.yml?

azure:
  store_build_artifacts: true

This would allow us to see what actually gets packaged (by downloading the zipped files from the CI).

ngam

Can you remove this the install step from the build script for now?

recipe/meta.yaml

ngam · 2022-08-14T13:48:48Z

Anyway, it is working as intended I believe, the key portion of the CI starts on line 845

Maybe a statement should be printed and a way to cancel...

Executing transaction: ...working...   Installing known registries into `~/feedstock_root/build_artifacts/pysr_1660455210381/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/share/julia`
  No Changes to `~/feedstock_root/build_artifacts/pysr_1660455210381/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/share/julia/environments/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/Project.toml`
  No Changes to `~/feedstock_root/build_artifacts/pysr_1660455210381/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/share/julia/environments/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/Manifest.toml`

Co-authored-by: ngam <[email protected]>

ngam · 2022-08-14T14:58:36Z

The install/build script(s) for pysr deleted the following files (from dependencies) from the prefix:
['lib/julia/libmbedtls.so', 'lib/julia/libnghttp2.so', 'lib/julia/libmbedx509.so', 'lib/julia/libcurl.so', 'lib/julia/libopenblas64_.so', 'lib/julia/libmbedcrypto.so']
This will cause the post-link checks to mis-report. Please try not to delete and files (DSOs in particular) from the prefix
WARNING:conda_build.build:The install/build script(s) for pysr deleted the following files (from dependencies) from the prefix:
['lib/julia/libmbedtls.so', 'lib/julia/libnghttp2.so', 'lib/julia/libmbedx509.so', 'lib/julia/libcurl.so', 'lib/julia/libopenblas64_.so', 'lib/julia/libmbedcrypto.so']
This will cause the post-link checks to mis-report. Please try not to delete and files (DSOs in particular) from the prefix

I wonder what exactly this means...

ngam · 2022-08-14T15:04:16Z

Btw, we need to edit your reqs --- something like this is enough I believe or along the lines of what isuruf suggested: https://github.com/conda-forge/staged-recipes/pull/17647/files

requirements:
  host:
    - pip
    - python >=3.7
  run:
    - pyjulia >=0.5.7
    - julia <=1.7.1
    - numpy
    - pandas
    - python >=3.7
    - scikit-learn >=1.0.0
    - sympy

conda-forge-linter · 2022-08-14T15:28:16Z

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

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

The top level meta key azure is unexpected

…stock into post-link

conda-forge-linter · 2022-08-14T15:29:38Z

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.

MilesCranmer · 2022-08-14T15:36:17Z

Thanks, all suggestions implemented. Let's see if it works

MilesCranmer · 2022-08-14T15:58:00Z

Since the deps are changing, do I need to merge a new version of PySR (i.e., creating a v0.10.1), or is updating the build number enough?

ngam · 2022-08-14T16:46:20Z

build number is enough

ngam · 2022-08-14T16:48:24Z

recipe/meta.yaml

    - python >=3.6,<4.0
    - pyjulia >=0.5.7
    - julia <=1.7.1
    - numpy >=1.14.0
    - pandas
    - sympy
-    - scikit-learn
-    - openlibm
+    - scikit-learn >=1.0.0
+    - openlibm <0.8.0


Is it still the case that your package doesn't support 1.7.2? We have 1.8 coming up soon too.

Anyway, if the <=1.7.2 isn't strictly necessary, I think we can get rid of julia and openlibm from here. You get julia from pyjulia anyway, and openlibm comes with julia.

PySR itself works for any Julia version above 1.5.

I saw issues with the conda-forge version of julia 1.7.2 when I tried it a while ago, and since most PySR users don't use Julia for other things I thought it would be fine to fix it at a stable version. I could try relaxing that requirement now.

Yeah, sounds good. It is a good exercise to relax it to see if things work as expected. I believe we implemented significant changes in 1.7.2 that will carry over to 1.8.0, so it is good to resolve this at some point, though not necessarily in this PR

ngam · 2022-08-14T16:51:16Z

My one last concern is that we need to get someone from @conda-forge/core to approve this to avoid misunderstanding and trouble. Maybe @isuruf? I would give them like a week to respond.

If no response, my opinion is: You're the maintainer here, and this is your package after all, so you should feel empowered to make whatever changes that you think are appropriate and beneficial.

MilesCranmer · 2022-08-14T16:59:00Z

Sounds good to me. If there's no response this week I will plan to merge this next weekend.

Also: I relaxed the Julia version requirement to 1.7.*.

isuruf · 2022-08-14T17:00:09Z

recipe/post-link.sh

@@ -0,0 +1 @@
+"${PREFIX}/bin/python" -c 'import pysr; pysr.install()' >> "${PREFIX}/.messages.txt" 2>&1


Where would this install the julia packages?

$CONDA_PREFIX/share I believe, especially after we specified the post-links in the julia-feedstock.

In theory, right here:
https://github.com/conda-forge/julia-feedstock/blob/b87f21cda4539b3069a369ca28e3379054eacdd1/recipe/scripts/activate.sh#L5-L6

@ngam I'm assuming the packages get installed in a location according to this env variable, right?

Oops, messages crossed :)

If a downstream package uses pysr when being built by conda-build, will conda-build package the julia packages installed by this post-link script?

Yep, it will certainly do (I think). Is there anyway to limit this? Make it into an activate script?

I thought post-link.sh runs on the user side?

Yes, but the user can be the package building process. If you get pysr in your host or build reqs, then it will activate and do the whole post-link during the build.

I think we can defend against this by checking whether CONDA_BUILD_STATE is not test in the post-link script.

ngam · 2022-08-14T17:07:00Z

Hm... @MilesCranmer

[ Info: Trying to import PyCall...
┌ Error: `import PyCall` failed
│   exception =
│    ArgumentError: Package PyCall [438e738f-606a-5dbb-bf0a-cddfbfd45ab0] is required but does not seem to be installed:
│     - Run `Pkg.instantiate()` to install all recorded dependencies.
│    
│    Stacktrace:
│     [1] _require(pkg::Base.PkgId)
│       @ Base ./loading.jl:1089
│     [2] require(uuidkey::Base.PkgId)
│       @ Base ./loading.jl:1013
│     [3] top-level scope
│       @ ~/feedstock_root/build_artifacts/pysr_1660496397213/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/lib/python3.10/site-packages/julia/install.jl:36
│     [4] include(mod::Module, _path::String)
│       @ Base ./Base.jl:418
│     [5] exec_options(opts::Base.JLOptions)
│       @ Base ./client.jl:292
│     [6] _start()
│       @ Base ./client.jl:495

What went wrong between 1.7.1 and 1.7.2+?

MilesCranmer · 2022-08-14T17:08:51Z

@ngam that issue by itself is fine - that's how the normal PyJulia install process works - it installs PyCall.jl if it can't import it.

However, the other issues look to be new: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=551157&view=logs&j=656edd35-690f-5c53-9ba3-09c10d0bea97&t=e5c8ab1d-8ff9-5cae-b332-e15ae582ed2d&l=683 from julia >1.7.1

ngam · 2022-08-14T17:29:06Z

@conda-forge-admin, please rerender

…nda-forge-pinning 2022.08.14.15.51.06

ngam · 2022-08-14T17:33:41Z

@ngam that issue by itself is fine - that's how the normal PyJulia install process works - it installs PyCall.jl if it can't import it.

However, the other issues look to be new: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=551157&view=logs&j=656edd35-690f-5c53-9ba3-09c10d0bea97&t=e5c8ab1d-8ff9-5cae-b332-e15ae582ed2d&l=683 from julia >1.7.1

I think the issue is with pycall still:

  [8e850b90] + libblastrampoline_jll
  [8e850ede] + nghttp2_jll
    Building Conda ─→ `~/feedstock_root/build_artifacts/pysr_1660496397213/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/share/julia/scratchspaces/44cfe95a-1eb2-52ea-b672-e2afdf69b78f/6e47d11ea2776bc5627421d59cdcc1296c058071/build.log`
    Building PyCall → `~/feedstock_root/build_artifacts/pysr_1660496397213/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/share/julia/scratchspaces/44cfe95a-1eb2-52ea-b672-e2afdf69b78f/1fc929f47d7c151c839c5fc1375929766fb8edcc/build.log`
ERROR: LoadError: Error building `PyCall`: 

DirectoryNotACondaEnvironmentError: The target directory exists, but it is not a conda environment.
Use 'conda create' to convert the directory to a conda environment.
  target directory: /home/conda/feedstock_root/build_artifacts/pysr_1660496397213/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p


[ Info: Running `conda install -y numpy` in root environment

ngam · 2022-08-14T17:38:44Z

I think the issue here is that your package is trying to make another conda env for itself. Currently, what we are doing is stacking envs. You have a conda env, right? Then a julia env gets built within it inside $CONDA_PREFIX/share/julia/ so there is no longer a need to create a new conda env and instead one can work within the existing conda env in the hierarchical structure.

I think this makes sense as the change from julia 1.7.1 to 1.7.3 exposed this issue. In your helper function, it may help to adjust it so that it can make use of this new feature we introduced --- i.e., all you need to do is ask julia to install your deps directly. What does PyCall do exactly in this case btw?

MilesCranmer · 2022-08-14T17:51:27Z

pysr.install() is doing the following:

Install PyCall.jl using PyJulia's install() command. Code
Pkg.activate("pysr-0.10.0", shared=true) - (This should create a new env). Code
Pkg.add("SymbolicRegression"). Code
Pkg.instantiate(). Code

ngam

This isn't working well. The issue raised by isuruf is crucial --- if another package uses pysr at some point we will end up packaging all these julia modules automatically, which we do not want.

I think we will have to settle with a user-triggered solution...

ngam · 2022-08-14T17:54:45Z

pysr.install() is doing the following:

Install PyCall.jl using PyJulia's install() command. Code

Pkg.activate("pysr-0.10.0", shared=true) - (This should create a new env). Code

Pkg.add("SymbolicRegression"). Code

Pkg.instantiate(). Code

Yes, I am not sure what's causing the error. I got it to work fine with a separate step after the fact (not post-link, i.e. like what you had before in the tests):

     Cloning git-repo `https://github.com/JuliaParallel/ClusterManagers.jl`
   Resolving package versions...
    Updating `~/.Mambaforge-Linux-x86_64/envs/py310/share/julia/environments/pysr-0.10.0/Project.toml`
  [34f1f09b] + ClusterManagers v0.4.2 `https://github.com/JuliaParallel/ClusterManagers.jl#14e7302`
    Updating `~/.Mambaforge-Linux-x86_64/envs/py310/share/julia/environments/pysr-0.10.0/Manifest.toml`
  [34f1f09b] + ClusterManagers v0.4.2 `https://github.com/JuliaParallel/ClusterManagers.jl#14e7302`
Precompiling project...
  82 dependencies successfully precompiled in 205 seconds (8 already precompiled)
/home/ngam/Repos/PySR/pysr/julia_helpers.py:39: UserWarning: It is recommended to restart Python after installing PySR's dependencies, so that the Julia environment is properly initialized.
  warnings.warn(

ocefpaf · 2022-08-22T13:31:16Z

recipe/meta.yaml

@@ -10,33 +10,21 @@ source:

 build:
  noarch: python


The package probably cannot be noarch now, right? Will we need post-link.bat?

julia-feedstock is only available on macOS and linux so I'm assuming we can leave it as noarch until that changes?

It is better the opposite in that case b/c these noarch packages will be available when the Julia package lands on those platforms, creating possible solutions but faulty installations.

ngam · 2022-08-28T22:26:41Z

@mkitti could you have a look at what went wrong here? I am not entirely sure...

Create post-link script to install Julia deps

ca3a0e0

Fix install command in post-link

44c473f

ngam reviewed Aug 14, 2022

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

Remove unnecessary install step

c24c23d

Co-authored-by: ngam <[email protected]>

Store build artifacts

36ec05f

Merge branch 'post-link' of https://github.com/MilesCranmer/pysr-feed…

748c462

…stock into post-link

MilesCranmer force-pushed the post-link branch from 36b8430 to 748c462 Compare August 14, 2022 15:29

Clean up redundant requirements

1851860

Force scikit-learn version

30e7480

Add smoke test for running PySRRegressor

1aa9822

ngam reviewed Aug 14, 2022

View reviewed changes

MilesCranmer added 2 commits August 14, 2022 12:54

Remove julia upper bound

0d57c39

Add 1.8.0 upper bound to Julia requirement

207d302

isuruf reviewed Aug 14, 2022

View reviewed changes

MNT: Re-rendered with conda-build 3.22.0, conda-smithy 3.21.1, and co…

c142311

…nda-forge-pinning 2022.08.14.15.51.06

MilesCranmer mentioned this pull request Aug 14, 2022

Integration test using downstream PyJulia packages conda-forge/julia-feedstock#215

Closed

ngam suggested changes Aug 14, 2022

View reviewed changes

ngam mentioned this pull request Aug 19, 2022

let's improve this recipe (and set a standard for python packages calling julia packages) #38

Closed

1 task

ocefpaf reviewed Aug 22, 2022

View reviewed changes

ngam mentioned this pull request Aug 23, 2022

Add xbitinfo conda-forge/staged-recipes#20116

Merged

10 tasks

mkitti mentioned this pull request Sep 12, 2022

v0.10.4 with new build strategy #43

Merged

5 tasks

MilesCranmer closed this Sep 20, 2022

		@@ -0,0 +1 @@
		"${PREFIX}/bin/python" -c 'import pysr; pysr.install()' >> "${PREFIX}/.messages.txt" 2>&1

Create post-link script to install Julia deps #41

Create post-link script to install Julia deps #41

Conversation

MilesCranmer commented Aug 14, 2022

conda-forge-linter commented Aug 14, 2022

ngam commented Aug 14, 2022 • edited Loading

ngam left a comment

Choose a reason for hiding this comment

ngam commented Aug 14, 2022 • edited Loading

ngam commented Aug 14, 2022

ngam commented Aug 14, 2022

conda-forge-linter commented Aug 14, 2022

conda-forge-linter commented Aug 14, 2022

MilesCranmer commented Aug 14, 2022

MilesCranmer commented Aug 14, 2022

ngam commented Aug 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngam commented Aug 14, 2022

MilesCranmer commented Aug 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngam commented Aug 14, 2022 • edited Loading

MilesCranmer commented Aug 14, 2022

ngam commented Aug 14, 2022

ngam commented Aug 14, 2022

ngam commented Aug 14, 2022

MilesCranmer commented Aug 14, 2022

ngam left a comment

Choose a reason for hiding this comment

ngam commented Aug 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngam commented Aug 28, 2022

ngam commented Aug 14, 2022 •

edited

Loading

ngam commented Aug 14, 2022 •

edited

Loading

ngam commented Aug 14, 2022 •

edited

Loading