Factor out PyTorch device setting

[kuutsav]

Addresses #493

[ascillitoe]

Hi @kuutsav, welcome! and thanks for contributing 🙂

[ascillitoe]

Thanks for this @kuutsav. The overall strategy looks good to me (assuming you are OK with the get_torch_device function name and location @arnaudvl?).

There are a few issues that need addressing though:

• The flake8 linting issues. See CONTRIBUTING.md for more guidance, but essentially if you run pip install -r requirements/dev.txt you can then run flake8 alibi_detect and mypy alibi_detect to check these both pass locally before pushing.
• The new function can also be added to pytorch/learned_kernel.py and pytorch/classifier.py. (You've correctly left out pytorch/spot_the_diff.py, the logic is different for this one).

[arnaudvl]

Thanks for the contribution @kuutsav . Just left a small comment which is applicable to the various detectors.

[kuutsav]

Thanks for the contribution @kuutsav . Just left a small comment which is applicable to the various detectors.

@arnaudvl Have addressed the issue along with the linting issue. Have also added the method at pytorch/learned_kernel.py and pytorch/classifier.py.

Do we want to print or log here?
print('No GPU detected, fall back on CPU.')

[ascillitoe]

Do we want to print or log here?
print('No GPU detected, fall back on CPU.')

Good question. IMO we should stick with logging.

[ascillitoe]

LGTM! Would be grateful if you could fix the 3 minor comment typo's I've just spotted (these were already there)

I'll also leave @arnaudvl to review one more time.

[arnaudvl]

One more place where it makes sense to use this utility function is the alibi_detect.utils.pytorch.predict_batch functionality. Note that the input argument type hint would need to be updated as well. Besides my comments, looks good!

[kuutsav]

One more place where it makes sense to use this utility function is the alibi_detect.utils.pytorch.predict_batch functionality. Note that the input argument type hint would need to be updated as well. Besides my comments, looks good!

Made this change as well. Had to ignore the types at couple of places as mypy was complaining due to reassignment of device.

[ascillitoe]

Sorry @kuutsav, after looking at this a bit more (after the most recent mypy errors), I've realised the current changes to utils/pytorch/prediction.py are not quite right.

Changing device: torch.device to device: Optional[str] will raise more issues since device is type torch.device in predict_batch_transformer (which calls predict_batch), and in cd.preprocess.preprocess_drift (which calls both of these functions).

Instead of adding multiple (i.e. 5 or 6) type: ignore's, I've instead tweaked get_device so that it can take in a torch.device and simply return it untouched, for compatibility with predict_batch. @arnaudvl what do you think? I'll wait to get your thoughts before reviewing again...

[ascillitoe]

LGTM, but will leave @arnaudvl to do a final pass since I'm now self-reviewing...

[ascillitoe]

Congrats (and thanks) on your first contribution @kuutsav! 🎉