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

numba v0.56.2 #94

Merged
merged 15 commits into from
Sep 28, 2022
Merged

Conversation

regro-cf-autotick-bot
Copy link
Contributor

@regro-cf-autotick-bot regro-cf-autotick-bot commented Jul 25, 2022

It is very likely that the current package version for this feedstock is out of date.

Checklist before merging this PR:

  • Dependencies have been updated if changed: see upstream
  • Tests have passed
  • Updated license if changed and license_file is packaged

Information about this PR:

  1. Feel free to push to the bot's branch to update this PR if needed.
  2. The bot will almost always only open one PR per version.
  3. The bot will stop issuing PRs if more than 3 version bump PRs generated by the bot are open. If you don't want to package a particular version please close the PR.
  4. If you want these PRs to be merged automatically, make an issue with @conda-forge-admin,please add bot automerge in the title and merge the resulting PR. This command will add our bot automerge feature to your feedstock.
  5. If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

Dependency Analysis

We couldn't run dependency analysis due to an internal error in the bot. :( Help is very welcome!

This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/autotick-bot/actions/runs/2735386165, please use this URL for debugging.


Closes #98
Closes #99
Closes #101

@conda-forge-linter
Copy link

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.

@henryiii
Copy link
Contributor

I’ve been traveling for a few weeks (including SciPy) and should be back at a computer tomorrow!

@jakirkham
Copy link
Member

No worries. Also we probably need the llvmlite update to proceed ( conda-forge/llvmlite-feedstock#62 ), which is blocked atm due to Windows issues.

@jakirkham
Copy link
Member

@conda-forge-admin, please re-render

@jakirkham
Copy link
Member

Now that llvmlite is out, have updated this PR with the latest version constraints. Though there were a few changes. So would be good to have a few more eyes on it 🙂

@jakirkham
Copy link
Member

cc @stuartarchibald (for awareness)

@jakirkham
Copy link
Member

Note: The aarch64 builds are currently timing out.

@hmaarrfk
Copy link
Contributor

It doesn't seem like the yare timing out as much as just using too many cores and crashing the tests.

recipe/meta.yaml Outdated Show resolved Hide resolved
@henryiii henryiii mentioned this pull request Aug 14, 2022
1 task
@henryiii
Copy link
Contributor

Looks like this is segfaulting on all CPython on ARM?

@jakirkham
Copy link
Member

Should add llvmlite in conda-forge uses shared libraries from LLVM. So we shouldn't need to rebuild it. Just need to rebuild LLVM (insert appropriate LOTR meme)

@hmaarrfk
Copy link
Contributor

Seems that applying that patch is not ABI compatibile.

@stuartarchibald
Copy link
Contributor

Seems that applying that patch is not ABI compatibile.

Does the ABI incompatibility only concern LLVM 12+, whereas Numba needs LLVM 11?

@hmaarrfk
Copy link
Contributor

FYI, i think the meat of the discussion about ABI breakage is happening: conda-forge/llvmdev-feedstock#167 (review)

This was referenced Sep 2, 2022
@njriasan
Copy link

njriasan commented Sep 6, 2022

Is there any update on this release?

@stuartarchibald
Copy link
Contributor

Is there any update on this release?

I think it's waiting on resolving issues in the llvmdev package for LLVM 11 on aarch64.

@raybellwaves
Copy link
Member

latest may be related to this PR: conda-forge/llvmdev-feedstock#171

@raybellwaves raybellwaves mentioned this pull request Sep 19, 2022
1 task
@hmaarrfk
Copy link
Contributor

It is mostly waiting on final review from: conda-forge/llvmdev-feedstock#170

@jakirkham
Copy link
Member

@conda-forge-admin, please re-render


run:
- python
- {{ pin_compatible('llvmlite', max_pin='x.x') }}
- {{ pin_compatible('numpy') }}
- importlib-metadata # [py < 39]
# needed for pkg_resources
- setuptools
Copy link
Member

Choose a reason for hiding this comment

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

Noticed setuptools is constrained to <60 in requirements. Why was this needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Originally we noticed that there was a problem with setuptools 65 (Numba issue: numba/numba#8355). NumPy produced a deprecation warning stating:

  `numpy.distutils` is deprecated since NumPy 1.23.0, as a result
  of the deprecation of `distutils` itself. It will be removed for
  Python >= 3.12. For older Python versions it will remain present.
  It is recommended to use `setuptools < 60.0` for those Python versions.
  For more details, see:
    https://numpy.org/devdocs/reference/distutils_status_migration.html 

This is why we constrained it to <60. However, this does cause other issues, as noted in numba/numba#8366 (comment).

The plan is to make setuptools an optional runtime dependency in Numba, and not pin to any specific version (see numba/numba#8366 (comment)) - so I think it will be OK not to constrain to any specific setuptools version, as long as the packages build successfully.

Copy link
Member

@jakirkham jakirkham Sep 28, 2022

Choose a reason for hiding this comment

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

Gotcha. Maybe we can add the constraint to host.

It looks like outside of building, setuptools is only really used in tests. Is that right? If so, maybe we can add the constraint to test requirements.

Wondering if we can drop setuptools from run. Or at least not add the constraint there (as that may be disruptive to users).

Edit: Should add the comment above suggests setuptools was needed for pkg_resources. Though am not seeing pkg_resources used in Numba atm. Is that correct or am I missing something?

Copy link
Contributor

Choose a reason for hiding this comment

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

setuptools is needed by the pycc tool, so it needs to be installed alongside Numba, otherwise someone who installs Numba and tries to run pycc for ahead-of-time compilation will run into an issue - setuptools still needs to be part of run.

I think pkg_resources is not used in Numba at present.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha. Is pycc using numpy.distutils or just distutils (and setuptools ofc)?

Copy link
Member

Choose a reason for hiding this comment

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

As we are diverging here now from the official package constraints, this sadly causes all packages that depend on numba to have a failing pip check. I would argue that we should align with upstream here. If numba (maintainers) thinks this is not an issue, then it should be removed upstream (as done in one of the linked PRs which probably takes a bit longer). As long as upstream defines this constraint and it is in the pip metadata, we should reflect that in the conda metadata. I went ahead and made a PR to address that: #104

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should match NumPy (in Numba upstream). NumPy does not require setuptools<60, so Numba should not either. It's really serious to limit to <60, since 61-62 added pyproject.toml config, so I'm expecting to see a growing number of setuptools minimums around that level.

Also, there's a pretty trivial fix for setuptools >60. It's not a real requirement.

Why does our pip check pass but not others?

Copy link
Contributor

Choose a reason for hiding this comment

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

I do see it here. 😱

Copy link
Member

Choose a reason for hiding this comment

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

Why does our pip check pass but not others?

Probably because there is a run_test.sh the entries in test: commands: … are not executed. I don't see it in the logs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Numba version 0.56.3 has been released with no restrictions on the setuptools version so as to address this issue. Thanks for the feedback.

@jakirkham jakirkham changed the title numba v0.56.0 numba v0.56.2 Sep 28, 2022
@jakirkham
Copy link
Member

With PR ( conda-forge/llvmdev-feedstock#171 ) in, refreshed this PR. Updated to the latest version and updated requirements. There was a setuptools version constraint that I was unsure about. Asked about this above. Please take a look and let us know if anything else is needed here.

recipe/run_test.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@gmarkall gmarkall left a comment

Choose a reason for hiding this comment

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

I think this is looking good now:

  • The AArch64 consecutive registers patch in llvmdev appears to have resolved the AArch64 issues.
  • I don't think the setuptools pin is necessary (and it would indeed be problematic to pin it in other ways)

Copy link
Contributor

@henryiii henryiii left a comment

Choose a reason for hiding this comment

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

Looks good, and agree about avoiding a runtime pin on setuptoools.

@jakirkham jakirkham merged commit 4794b31 into conda-forge:main Sep 28, 2022
@jakirkham
Copy link
Member

Thanks all! 🙏 Let's get these packages out :shipit:

@regro-cf-autotick-bot regro-cf-autotick-bot deleted the 0.56.0_h5e4d67 branch September 28, 2022 17:13
@jakirkham
Copy link
Member

jakirkham commented Sep 28, 2022

Looks like all of the ppc64le jobs on main timed out on CI. This wasn't an issue in the PR previously. Trying restarting them...

Edit: They are still failing. Raised issue ( #102 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants