Skip to content

Update some torchscript tests after #24505 - #24566

Merged
ydshieh merged 2 commits into
mainfrom
fix_torchscript_tests
Jun 29, 2023
Merged

Update some torchscript tests after #24505#24566
ydshieh merged 2 commits into
mainfrom
fix_torchscript_tests

Conversation

@ydshieh

@ydshieh ydshieh commented Jun 29, 2023

Copy link
Copy Markdown
Collaborator

What does this PR do?

Need to update the logic in some torchscript tests after #24505

@ydshieh
ydshieh requested a review from sgugger June 29, 2023 04:42
@HuggingFaceDocBuilderDev

HuggingFaceDocBuilderDev commented Jun 29, 2023

Copy link
Copy Markdown

The documentation is not available anymore as the PR was closed or merged.

@sgugger sgugger left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for updating those! Left a comment to replicate across the tests.

Comment thread tests/models/blip/test_modeling_blip.py
Comment thread tests/models/blip/test_modeling_blip.py
@ydshieh

ydshieh commented Jun 29, 2023

Copy link
Copy Markdown
Collaborator Author

@sgugger Thanks for the suggestion. I actually copied from the common test file. However, there is

            model_buffers = list(model.buffers())
            for non_persistent_buffer in non_persistent_buffers.values():
                found_buffer = False
                for i, model_buffer in enumerate(model_buffers):
                    if torch.equal(non_persistent_buffer, model_buffer):
                        found_buffer = True
                        break

                self.assertTrue(found_buffer)
                model_buffers.pop(i)

there (which I didn't see when working on this PR). This uses the values.

So I am going to copy the above block (and therefore keeping using dict). Let me know if you have other opinions instead.

@sgugger

sgugger commented Jun 29, 2023

Copy link
Copy Markdown
Collaborator

If you end up using the values, no worries.

@ydshieh

ydshieh commented Jun 29, 2023

Copy link
Copy Markdown
Collaborator Author

Copy the mentioned block to all _create_and_check_torchscript definition.

Let's not change this by looking if the model use persistent or non-persistent buffuer: just keep the logic in the common test file.

@ydshieh
ydshieh merged commit 77db28d into main Jun 29, 2023
@ydshieh
ydshieh deleted the fix_torchscript_tests branch June 29, 2023 14:05
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.

3 participants