Skip to content

ci: test with latest dependencies #2122

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

Merged
merged 25 commits into from
Jun 17, 2025
Merged

ci: test with latest dependencies #2122

merged 25 commits into from
Jun 17, 2025

Conversation

Borda
Copy link
Member

@Borda Borda commented May 22, 2025

Before submitting
  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure to update the docs?
  • Did you write any new necessary tests?

What does this PR do?

This addresses now user's scenario who install the package on blank environment

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

Sorry, something went wrong.

@Borda Borda requested review from lantiga and t-vi as code owners May 22, 2025 14:10
@github-actions github-actions bot added the ci label May 22, 2025
@Borda Borda requested a review from mruberry as a code owner May 22, 2025 14:20
Borda added 2 commits May 22, 2025 16:25
bitsandbytes>=0.45.2,<0.45.5; sys_platform!='darwin'
@Borda
Copy link
Member Author

Borda commented May 22, 2025

just for mac

l = <thunder.core.interpreter.WrappedValue object at 0x152a77460>

    def populate_item_wrappers(l):
        ctx: InterpreterCompileCtx = get_interpretercompilectx()
        if not ctx._with_provenance_tracking:
            return
    
        assert isinstance(l, WrappedValue)
        # to do: generalize
        if wrapped_isinstance(l, (list, tuple)):
            if l.item_wrappers is None:
                l.item_wrappers = [None for _ in range(len(l.value))]
            assert isinstance(l.item_wrappers, list)
            assert len(l.value) == len(l.item_wrappers), f"{len(l.value)=} {len(l.item_wrappers)=}"
    
            for i, v in enumerate(l.value):
                if l.item_wrappers[i] is None:
                    wv = wrap_binary_subscr(v, l, i)
                    l.item_wrappers[i] = wv
            return
    
        if wrapped_isinstance(l, dict):
            assert isinstance(l.item_wrappers, dict)
>           for k, v in l.value.items():
E           RuntimeError: dictionary changed size during iteration

thunder/core/interpreter.py:361: RuntimeError

@t-vi, any insides what is happening?

@Borda
Copy link
Member Author

Borda commented May 22, 2025

also on GPU:
FAILED thunder/tests/test_torch_compile_executor.py::test_torch_compile_cat_rope_single_fusion - ValueError: cos must be three-dimensional, but shape is (8192, 128)

@Borda Borda mentioned this pull request May 23, 2025
@kiya00 kiya00 mentioned this pull request May 30, 2025
4 tasks
@t-vi
Copy link
Collaborator

t-vi commented Jun 12, 2025

There is a race condition, I'm looking into fixing it, but I need to find the perf thing with it.

@Borda
Copy link
Member Author

Borda commented Jun 17, 2025

There is a race condition, I'm looking into fixing it, but I need to find the perf thing with it.

cool, thank you

Comment on lines +20 to +21
bitsandbytes>=0.44,<0.45; sys_platform!='darwin'
bitsandbytes>=0.42,<0.43; sys_platform=='darwin'
Copy link
Member Author

Choose a reason for hiding this comment

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

just relax it as a range 🐿️

Copy link
Collaborator

Choose a reason for hiding this comment

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

There doesn't seem to be such package for ARM linux, any idea about this?

#37 8.827 ERROR: Could not find a version that satisfies the requirement bitsandbytes<0.45,>=0.44 (from versions: 0.31.8, 0.32.0, 0.32.1, 0.32.2, 0.32.3, 0.33.0, 0.33.1, 0.34.0, 0.35.0, 0.35.1, 0.35.2, 0.35.3, 0.35.4, 0.36.0, 0.36.0.post1, 0.36.0.post2, 0.37.0, 0.37.1, 0.37.2, 0.38.0, 0.38.0.post1, 0.38.0.post2, 0.38.1, 0.39.0, 0.39.1, 0.40.0, 0.40.0.post1, 0.40.0.post2, 0.40.0.post3, 0.40.0.post4, 0.40.1, 0.40.1.post1, 0.40.2, 0.41.0, 0.41.1, 0.41.2, 0.41.2.post1, 0.41.2.post2, 0.41.3, 0.41.3.post1, 0.41.3.post2, 0.42.0, 0.46.0)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

weird it was downloading

#37 7.810 Collecting bitsandbytes==0.42.0 (from -r requirements/test.txt (line 20))
#37 7.814   Downloading bitsandbytes-0.42.0-py3-none-any.whl.metadata (9.9 kB)

two days ago on ARM linux

Copy link
Collaborator

Choose a reason for hiding this comment

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

First, apologies for breaking this for you.
I think we want "arm" not in platform_machine and "arm" in platform_machine as the conditions (cf. platform.machine() ). WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, that sounds right
also maybe we can ask bitsandbytes to compile also ARM packges?

Copy link
Member Author

Choose a reason for hiding this comment

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

ref: #2257

Copy link
Collaborator

Choose a reason for hiding this comment

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

update on this: since bitsandbytes's release binaries usually aren't built with the latest cuda versions, we're now building BNB from source with the latest cuda versions internally

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you going to publish it to nvidia pypi?

Copy link
Collaborator

@xwang233 xwang233 Jun 18, 2025

Choose a reason for hiding this comment

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

I'm not sure because those are dev cuda versions before public release. It probably wouldn't work for you without the cuda library?

@Borda Borda enabled auto-merge (squash) June 17, 2025 11:50
Copy link
Collaborator

@t-vi t-vi left a comment

Choose a reason for hiding this comment

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

Thank you @Borda

@Borda Borda merged commit 393277b into main Jun 17, 2025
51 checks passed
@Borda Borda deleted the ci/test-with-latest branch June 17, 2025 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants