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

Fix docs-check job; numpy 2.x fixes #157

Merged
merged 12 commits into from
Jun 25, 2024

Conversation

sk1p
Copy link
Member

@sk1p sk1p commented Jun 11, 2024

Try to be consistent with other jobs.

Contributor Checklist:

  • I have added or updated my entry in the creators.json file
  • I have added a changelog entry for my contribution
  • I have added/updated documentation for all user-facing changes
  • I have added/updated test cases

Reviewer Checklist:

  • /azp run libertem.libertem-live-data passed

@sk1p sk1p added the infra CI/CD infrastructure etc. label Jun 11, 2024
@sk1p
Copy link
Member Author

sk1p commented Jun 11, 2024

/azp run libertem.libertem-live-data

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link

codecov bot commented Jun 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.84%. Comparing base (3562273) to head (acae5ff).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #157   +/-   ##
=======================================
  Coverage   79.83%   79.84%           
=======================================
  Files          35       35           
  Lines        3457     3458    +1     
  Branches      576      576           
=======================================
+ Hits         2760     2761    +1     
  Misses        560      560           
  Partials      137      137           

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

@sk1p sk1p changed the title Run docs checks in venv, like other jobs WIP: fix docs-check job Jun 11, 2024
@sk1p

This comment was marked as outdated.

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sk1p

This comment was marked as outdated.

1 similar comment
@sk1p
Copy link
Member Author

sk1p commented Jun 11, 2024

/azp run libertem.libertem-live-data

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@matbryan52
Copy link
Member

/azp run libertem.libertem-live-data

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@matbryan52
Copy link
Member

matbryan52 commented Jun 21, 2024

I am genuinely perplexed about this, without installing uv or tox-uv, and forcing also tox env recreation the pipeline is still using uv to install packages, and I think this is referencing a py38 install which we never ask for.

If there is some kind of caching on the pipelines maybe good to reset everything? I don't think I have the access to do that myself.

The numba test troubles are equally odd, I tried on ptycho with the same python, environment (checked via diff), numpy 2, branch, and there is no trouble. It looks like it only affects 16 bits per pixel tests in merlin/.../test_encode_roundtrip..., independently of the encode/decode size. There aren't even any warnings emitted ! All I can guess is that it it due to numba caching somewhere?

@sk1p
Copy link
Member Author

sk1p commented Jun 25, 2024

I am genuinely perplexed about this, without installing uv or tox-uv, and forcing also tox env recreation the pipeline is still using uv to install packages,

This might be because the venv is not forcibly removed if it already exists, and tox-uv would still possibly be installed in there.

and I think this is referencing a py38 install which we never ask for.

Yeah, that's the really weird one. The Python 3.8 is just one of the versions that is made available to the azure agents. I'm not sure yet, and I have no idea why uv picks that one up! Maybe because that's the "native" version of the ubuntu:20.04 image we use as a base?

We might want to move to 22.04 as a base image for the azure agent, which does still support the Python versions we need, as far as I can see.

If there is some kind of caching on the pipelines maybe good to reset everything? I don't think I have the access to do that myself.
The numba test troubles are equally odd, I tried on ptycho with the same python, environment (checked via diff), numpy 2, branch, and there is no trouble. It looks like it only affects 16 bits per pixel tests in merlin/.../test_encode_roundtrip..., independently of the encode/decode size. There aren't even any warnings emitted ! All I can guess is that it it due to numba caching somewhere?

I didn't look further yet, but an issue is that the azure agent doesn't completely clean up the environment after a run, so there may indeed be some files that stick around (like tox environments, the venv, and the cache of pip/uv). I just re-created the containers, and I'll re-run it again, to see if that changed anything.

@sk1p
Copy link
Member Author

sk1p commented Jun 25, 2024

/azp run libertem.libertem-live-data

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sk1p
Copy link
Member Author

sk1p commented Jun 25, 2024

The numba test troubles are equally odd, I tried on ptycho with the same python, environment (checked via diff), numpy 2, branch, and there is no trouble. It looks like it only affects 16 bits per pixel tests in merlin/.../test_encode_roundtrip..., independently of the encode/decode size. There aren't even any warnings emitted ! All I can guess is that it it due to numba caching somewhere?

Hum... I can reproduce the failures - they only happen in the non-jit case (NUMBA_DISABLE_JIT=1 - via tox -e numba_coverage for example). I had to do some changes in LiberTEM itself for the offline decoding functions - probably there's an issue with changed dtype promotion rules (numba doesn't fully implement these yet, which is why they only fail with the jit disabled)

Hopefully that will be cleaned up by the agent
@sk1p
Copy link
Member Author

sk1p commented Jun 25, 2024

/azp run libertem.libertem-live-data

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@matbryan52
Copy link
Member

Hum... I can reproduce the failures - they only happen in the non-jit case (NUMBA_DISABLE_JIT=1 - via tox -e numba_coverage for example). I had to do some changes in LiberTEM itself for the offline decoding functions - probably there's an issue with changed dtype promotion rules (numba doesn't fully implement these yet, which is why they only fail with the jit disabled)

Ah I have just been running with pytest ... -m with_numba as I have found tox difficult when I try to run it myself. So the jit'd functions pass the tests but not the python-only form 🤔

@sk1p
Copy link
Member Author

sk1p commented Jun 25, 2024

/azp run libertem.libertem-live-data

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sk1p
Copy link
Member Author

sk1p commented Jun 25, 2024

/azp run libertem.libertem-live-data

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sk1p
Copy link
Member Author

sk1p commented Jun 25, 2024

/azp run libertem.libertem-live-data

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Numba and numpy 2.x don't yet agree on the dtype conversion rules, so
for disabled numba, we need to explicitly upcast to np.uint16 at some
places.
@sk1p sk1p changed the title WIP: fix docs-check job Fix docs-check job; numpy 2.x fixes Jun 25, 2024
@sk1p
Copy link
Member Author

sk1p commented Jun 25, 2024

/azp run libertem.libertem-live-data

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@matbryan52
Copy link
Member

matbryan52 commented Jun 25, 2024

So weird that uv is fixated wrong python. Otherwise well done, LGTM!

@matbryan52 matbryan52 merged commit a97f501 into LiberTEM:master Jun 25, 2024
31 checks passed
@sk1p sk1p deleted the docs-check-in-venv branch June 25, 2024 15:32
@sk1p
Copy link
Member Author

sk1p commented Jun 25, 2024

So weird that uv is fixated wrong python. Otherwise well done, LGTM!

Yeah, extra weird that it works fine for the other jobs, like data_tests or check_example_notebooks 🤷

@sk1p sk1p added this to the 0.3 milestone Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra CI/CD infrastructure etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants