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

Turn off numba cache by default #1191

Merged
merged 2 commits into from
Feb 16, 2024

Conversation

jeromekelleher
Copy link
Collaborator

Partial fix for #1156

This turns off numba caching by default, and at least lets us use the current git version of sgkit without hitting this issue repeatedly.

To fully address I think we probably need to undocument this variable by removing the mention to it in the docs, and maybe add some comments in the source laying out the issue.

Copy link
Collaborator

@tomwhite tomwhite left a comment

Choose a reason for hiding this comment

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

LGTM

@jeromekelleher
Copy link
Collaborator Author

Hmm, it seems like there's an unfortunate side effect here of turning off caching. With caching turned off, when we run the functions with NUMBA_DISABLE_JIT=1 to get coverage we get a whole bunch of numba errors and deprecations that don't happen when caching is on.

I'm not sure what to do here, there's quite a few deprecations and errors that seem non-trival to fix. For example, we seem to hit this problem numba/numba#8886 which is non-trival to workaround.

Any thoughts @tomwhite, @timothymillar?

@jeromekelleher
Copy link
Collaborator Author

I worked around this here by explicitly turning on caching, so that we can get our builds to pass and at least have our code not segfault by default. Opened #1194 to track the CI issue.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a386541) 100.00% compared to head (5c85a92) 100.00%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1191   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           52        52           
  Lines         5210      5210           
=========================================
  Hits          5210      5210           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jeromekelleher jeromekelleher added the auto-merge Auto merge label for mergify test flight label Feb 16, 2024
@mergify mergify bot merged commit d32b871 into sgkit-dev:main Feb 16, 2024
9 checks passed
@jeromekelleher jeromekelleher deleted the turnoff-numba-cache# branch February 16, 2024 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge Auto merge label for mergify test flight
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants