skorch
skorch copied to clipboard
Loading extra arguments w/ cuda dependency on CPU
Supersedes #877
Description
This bug could occur if a user has set a parameter with a CUDA dependency and then tries to load the net without CUDA. Now, this works (again) as expected.
Details
The problem started occurring after PR #751, which introduced storing
parameters set via set_params
in the private attribute _kwargs
.
Normally, for attributes, we make sure that they can be loaded without
CUDA, but attributes within _kwargs
were not checked. Thus, loading
those without CUDA failed. Unfortunately, this was not caught by CI
because CI is not CUDA-enabled.
Implementation
The bugfix consists of making sure that we don't store any values in
_kwargs
. Since values are not needed, only the keys (parameter names),
this is more efficient anyway. Thus, there are no more possibly
CUDA-dependent values that can "slip through".
After discussion, we decided to also rename the attribute, as _kwargs
was not very specific. The new attribute is called _params_to_validate
and it is a set instead of a dict. Also, the _check_kwargs
method was
renamed to _validate_params
and it doesn't take a kwargs
argument
anymore. And on top of that, it's changed the raised error from TypeError
to ValueError
.
The reason for making those change is that it now is similar to sklearn's
_validate_params
method on BaseEstimator
(same signature and same error
type). However, we don't make use of the actual sklearn machinery since
our validation does a few things differently (e.g. proposing possible
fixes when the name is wrong).
As the attribute was renamed, we would normally get an error when
unpickling nets stored with the old attribute. To prevent this, we catch
the old attribute _kwargs
and convert it to the new attribute
_params_to_validate
.
Coincidental changes
- Moved an entry in CHANGES.md to a different section
- Added a reference to an existing entry in CHANGES.md
- I adapted the code in
hf.py
to use the same new scheme