Skip to content
This repository was archived by the owner on Jul 1, 2025. It is now read-only.

update pybind#5067

Closed
yinghai wants to merge 2 commits intomasterfrom
xx
Closed

update pybind#5067
yinghai wants to merge 2 commits intomasterfrom
xx

Conversation

@yinghai
Copy link
Copy Markdown
Contributor

@yinghai yinghai commented Nov 11, 2020

Summary:
Recent update of pybind version from PyTorch (pytorch/pytorch#46415) created compatibility issue, resulting in mysterious errors like

__________________ TestRemoveException.test_remove_exceptions __________________
self = <tests.functionality.remove_exceptions_test.TestRemoveException testMethod=test_remove_exceptions>

    def test_remove_exceptions(self):
        """Test Glow's removeExceptions JIT pass"""
    
        foo_jit = torch.jit.script(foo)
        graph = foo_jit.graph
        assert graph_contains_str(graph, "prim::RaiseException")
>       torch_glow.removeExceptions_(graph)
E       TypeError: removeExceptions_(): incompatible function arguments. The following argument types are supported:
E           1. (arg0: torch::jit::Graph) -> None

This PR aligns the pybind11 versions in glow and pytorch, hence fixing the issue.

Documentation:

[Optional Fixes #issue]

Test Plan:

CI

Copy link
Copy Markdown
Contributor

@zrphercule zrphercule left a comment

Choose a reason for hiding this comment

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

Let's wait and see if PyTorch is fixed.

Copy link
Copy Markdown

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@yinghai has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Copy Markdown

@yinghai merged this pull request in 7c7d78d.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants