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

[BugFix] Gracefully handle C++ import error in TorchRL #1640

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

vmoens
Copy link
Contributor

@vmoens vmoens commented Oct 23, 2023

Description

Allows to import torchrl when _torchrl.so isn't available.

Wdyt @matteobettini ?

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 23, 2023
@vmoens vmoens added the quality code quality label Oct 23, 2023
@matteobettini
Copy link
Contributor

Why not an error? As I understand it, if that import fails, TorchRL is not installed as it should, so having a warning may just push errors down the line.

In any case I agree that there should be a custom message printed with instructions.

@vmoens
Copy link
Contributor Author

vmoens commented Oct 23, 2023

There's already an error, the goal is to be able to use the other 99.9% of the lib if for some reason the built went wrong. It's a recurrent issue on gh and most of the time I think people would be happy to use the lib as is.
For instznce, you could install torchrl with torch 2.1 and then downgrade torch and still have most of it working. Now just importing the lib will fail in that case and you'll need a local built (which requires ninja, gcc etc)

It doesn't come for free as you say but at least people are informed that some features are not available.

@matteobettini
Copy link
Contributor

Sure then makes sense.

Hopefully once we identify the cause it will be less relevant

@vmoens
Copy link
Contributor Author

vmoens commented Oct 23, 2023

Somewhat, though this is orthogonal to the M1 issue (which I will solve separately).

The lib is made to be mostly python-based so the few C++ classes we have should not block users from using the lib. Same goes with pytorch version: we allow different versions of pytorch to be used but we don't make the life of users who want to use torchrl with these easy.

@vmoens vmoens merged commit 8d2bc8b into main Oct 23, 2023
53 of 59 checks passed
@vmoens vmoens deleted the graceful_load_error branch October 23, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. quality code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants