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] fix issue with BloomBlock due to transformers upgrade #2640

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

siddvenk
Copy link
Contributor

@siddvenk siddvenk commented Dec 17, 2024

Description

While working on #2639, I noticed an issue with upgrading the transformers version for unit tests:

======================================================================
ERROR: test_contrastive_scheduler (djl_python.tests.test_scheduler_bloom.TestSchedulerBloom.test_contrastive_scheduler)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/djl-serving/djl-serving/engines/python/setup/djl_python/tests/test_scheduler_bloom.py", line 80, in test_contrastive_scheduler
    scheduler.add_request(input_ids_0, request_ids)
  File "/home/runner/work/djl-serving/djl-serving/engines/python/setup/djl_python/seq_scheduler/seq_batch_scheduler.py", line 116, in add_request
    self._add_request(input_ids[index_not_use_prompt],
  File "/home/runner/work/djl-serving/djl-serving/engines/python/setup/djl_python/seq_scheduler/seq_batch_scheduler.py", line 159, in _add_request
    new_seq_batcher, output_ids = seq_batcher_cls.init_forward(
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/hostedtoolcache/Python/3.11.11/x64/lib/python3.11/site-packages/torch/utils/_contextlib.py", line 116, in decorate_context
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/djl-serving/djl-serving/engines/python/setup/djl_python/seq_scheduler/seq_batcher_impl.py", line 192, in init_forward
    lm_output = lm_block.forward(*model_input, past_key_values=kv_cache)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/djl-serving/djl-serving/engines/python/setup/djl_python/seq_scheduler/lm_block.py", line 128, in forward
    _, kv_dim, seq_len = past_key_values[0][0].shape
    ^^^^^^^^^^^^^^^^^^
ValueError: too many values to unpack (expected 3)

I tried a few things to fix this, but ended up opting for the suggestion from transformers here https://huggingface.co/docs/transformers/en/kv_cache#legacy-cache-format. The rest of the code relies on the legacy cache format, so I convert to/from within this to keep the rest of the code as is.

After this change, I ran the full test suite with results:

----------------------------------------------------------------------
Ran 133 tests in 36.353s

OK (skipped=27)

Type of change

Please delete options that are not relevant.

  • [x ] Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist:

  • Have you manually built the docker image and verify the change?
  • [ x] Have you run related tests? Check how to set up the test environment here; One example would be pytest tests.py -k "TestCorrectnessLmiDist" -m "lmi_dist"
  • [x ] Have you added tests that prove your fix is effective or that this feature works?
  • Has code been commented, particularly in hard-to-understand areas?
  • Have you made corresponding changes to the documentation?

Feature/Issue validation/testing

Please describe the Unit or Integration tests that you ran to verify your changes and relevant result summary. Provide instructions so it can be reproduced.
Please also list any relevant details for your test configuration.

In addition to fixing the issue mentioned previously, I validated that the outputs from this test match those from before this change, which gives confidence since we're using a real model in this test:

Before Change

root@c056c0e00e86:/workspace/djl-serving/engines/python/setup# python -m unittest -v djl_python.tests.test_scheduler_bloom.TestSchedulerBloom.test_contrastive_scheduler
test_contrastive_scheduler (djl_python.tests.test_scheduler_bloom.TestSchedulerBloom.test_contrastive_scheduler) ... /usr/local/lib/python3.11/dist-packages/transformers/models/bloom/modeling_bloom.py:838: FutureWarning: `position_ids` have no functionality in BLOOM and will be removed in v5.0.0. You can safely ignore passing `position_ids`.
  warnings.warn(

0: Memories follow me left and right. I can feel them moving around in my body, like they’re trying to tell me something about where I’m going. And then there’s the sound of their breath

1: When your legs don't work like they used to before And I can't sweep you off my lap But if you're still here I'll take care of it If you're gone, you'll never know I'm gonna stay with you forever And I can feel the

2: There's a time that I remember, when I did not know what it was like to live in this country.
And then there are people who do.
They don't even want me here anymore because they're afraid of losing

3: When your legs don't work properly, it may be time to consider exercising them again. You should start slowly and then increase the intensity as soon as possible. If you feel

4: There's a time when you need something special.
I don't know what you're talking about.
- You want me to tell her?
-No way.
I'm sorry, honey.

ok

----------------------------------------------------------------------
Ran 1 test in 3.411s

OK

After Change:

root@c056c0e00e86:/workspace/djl-serving/engines/python/setup# python -m unittest -v djl_python.tests.test_scheduler_bloom.TestSchedulerBloom.test_contrastive_scheduler
test_contrastive_scheduler (djl_python.tests.test_scheduler_bloom.TestSchedulerBloom.test_contrastive_scheduler) ... /usr/local/lib/python3.11/dist-packages/transformers/models/bloom/modeling_bloom.py:961: FutureWarning: `position_ids` have no functionality in BLOOM and will be removed in v5.0.0. You can safely ignore passing `position_ids`.
  warnings.warn(

0: Memories follow me left and right. I can feel them moving around in my body, like they’re trying to tell me something about where I’m going. And then there’s the sound of their breath

1: When your legs don't work like they used to before And I can't sweep you off my lap But if you're still here I'll take care of it If you're gone, you'll never know I'm gonna stay with you forever And I can feel the

2: There's a time that I remember, when I did not know what it was like to live in this country.
And then there are people who do.
They don't even want me here anymore because they're afraid of losing

3: When your legs don't work properly, it may be time to consider exercising them again. You should start slowly and then increase the intensity as soon as possible. If you feel

4: There's a time when you need something special.
I don't know what you're talking about.
- You want me to tell her?
-No way.
I'm sorry, honey.

ok

----------------------------------------------------------------------
Ran 1 test in 3.390s

OK

@siddvenk siddvenk marked this pull request as ready for review December 17, 2024 03:24
@siddvenk siddvenk requested review from zachgk and a team as code owners December 17, 2024 03:24
@siddvenk siddvenk force-pushed the fix-python-unit-tests branch from 9f88dd4 to 5be834f Compare December 17, 2024 17:53
@siddvenk siddvenk merged commit bffb5a0 into deepjavalibrary:master Dec 17, 2024
9 checks passed
@siddvenk siddvenk deleted the fix-python-unit-tests branch December 17, 2024 18:34
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.

4 participants