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

Don't fail when gpytorch import fails #913

Merged
merged 5 commits into from
Nov 17, 2022

Conversation

BenjaminBossan
Copy link
Collaborator

@BenjaminBossan BenjaminBossan commented Nov 8, 2022

This is mostly relevant for CI, as most users probably don't have gpytorch installed.

The problem is that some gpytorch versions now raise an AttributeError with some torch versions. Since we try to import gpytorch in utils.py and in test_probabilistic.py but only catch ImportError, the AttributeError will be raised. However, the presence of an incompatible gpytorch version should not preclude the usage of skorch.

In the gpytorch tests themselves, we already check for compatible versions and skip if they are not compatible. However, to check versions, we first need to successfully import gpytorch, which fails.

Here is an example of a job that currently fails because of this:

https://github.com/skorch-dev/skorch/actions/runs/3421070263/jobs/5696711066

The solution now is to catch AttributeErrors during gpytorch imports. For utils.py, we then treat it as if gpytorch were not installed, for the tests we skip. A disadvantage of this indiscriminate skipping is that it could happen that all versions are skipped and we won't notice since CI is green. I don't have a good solution for this.

This is mostly relevant for CI, as most users probably don't have
gpytorch installed. The problem is that some gpytorch versions raise an
AttributeError with some torch versions. Since we try to import gpytorch
in utils.py but only catch ImportError, the AttributeError will be
raised. However, the presence of an incompatible gpytorch version should
not preclude the usage of skorch.

In the gpytorch tests themselves, we already check for compatible
versions and skip if they are not compatible.
It is in fact possible to import it only locally with minimal
performance overhead and thus avoid exposing skorch to possible gpytorch
import issues. On top of that, the import can be commented out for now
because it is only needed in case multiclass classification with
gaussian processes will be supported.
It's been there long enough without changes.
@BenjaminBossan
Copy link
Collaborator Author

@ottonemo I made the changes as discussed. Please take another look.

@ottonemo ottonemo merged commit cc2d36f into master Nov 17, 2022
@BenjaminBossan BenjaminBossan deleted the bugfix-failing-gpytorch-install branch November 17, 2022 13:15
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.

2 participants