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

Bugfixes: Bugs stemming from the use of non-standard modules and criteria #927

Merged
merged 3 commits into from
Jan 3, 2023

Conversation

BenjaminBossan
Copy link
Collaborator

These bugfixes were originally provided in #912 but are now moved to their own PR.

  1. Inferring the predict nonlinearity made the assumption that a net.criterion_ exists. However, this might not always be the case. Now, this function works even if the criterion attribute has a different name. Moreover, when more than one criterion is defined, the identity function will be returned.
  2. The second bug is that in check_is_fitted, we made the check dependent on the module_ attribute. Again, we should not assume that it always exists, as users may define different names. Now, those custom names will be checked, and only if those don't exist is it assumed that the module_ attribute should exist.
  3. The helper.py module now defines an __all__ attribute. This seems to be necessary for sphinx to build documentation for objects that are imported to, but not defined in, helper.py. Specifically, this is the case for the parse_args CLI feature.
  4. Fixed a small bug in on_grad_computed, which has training=False as default argument but during training, it should be set to True. Thankfully, it had no consequences in our code since it's only used by GradientNormClipping, which doesn't check that argument.

These bugfixes were originally provided in #912 but are now moved to their own
PR.

1. Inferring the predict nonlinearity made the assumption that a net.criterion_
   exists. However, this might not always be the case. Now, this function works
   even if the criterion attribute has a different name. Moreover, when more
   than one criterion is defined, the identity function will be returned.
2. The second bug is that in check_is_fitted, we made the check dependent on the
   module_ attribute. Again, we should not assume that it always exists, as
   users may define different names. Now, those custom names will be checked,
   and only if those don't exist is it assumed that the module_ attribute should
   exist.
3. The helper.py module now defines an __all__ attribute. This seems to be
   necessary for sphinx to build documentation for objects that are imported to,
   but not defined in, helper.py. Specifically, this is the case for the
   parse_args CLI feature.
4. Fixed a small bug in on_grad_computed, which has training=False as default
   arg but during training, it should be set to True. Thankfully, it had no
   consequences in our code since it's only used by GradientNormClipping, which
   doesn't check that argument.
Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

LGTM

@BenjaminBossan BenjaminBossan merged commit 5c3e553 into master Jan 3, 2023
@BenjaminBossan BenjaminBossan deleted the bugfixes-non-standard-attribute-names branch January 3, 2023 10:17
BenjaminBossan added a commit that referenced this pull request Mar 17, 2023
…eria (#927)

These bugfixes were originally provided in #912 but are now moved to their own
PR.

1. Inferring the predict nonlinearity made the assumption that a net.criterion_
   exists. However, this might not always be the case. Now, this function works
   even if the criterion attribute has a different name. Moreover, when more
   than one criterion is defined, the identity function will be returned.
2. The second bug is that in check_is_fitted, we made the check dependent on the
   module_ attribute. Again, we should not assume that it always exists, as
   users may define different names. Now, those custom names will be checked,
   and only if those don't exist is it assumed that the module_ attribute should
   exist.
3. The helper.py module now defines an __all__ attribute. This seems to be
   necessary for sphinx to build documentation for objects that are imported to,
   but not defined in, helper.py. Specifically, this is the case for the
   parse_args CLI feature.
4. Fixed a small bug in on_grad_computed, which has training=False as default
   arg but during training, it should be set to True. Thankfully, it had no
   consequences in our code since it's only used by GradientNormClipping, which
   doesn't check that argument.
@BenjaminBossan BenjaminBossan mentioned this pull request May 8, 2023
BenjaminBossan added a commit that referenced this pull request May 17, 2023
Preparation for release of version 0.13.0

Release text:

The new skorch release is here and it has some changes that will be exiting for
some users.

- First of all, you may have heard of the [PyTorch 2.0
  release](https://pytorch.org/get-started/pytorch-2.0/), which includes the
  option to compile the PyTorch module for better runtime performance. This
  skorch release allows you to pass `compile=True` when initializing the net to
  enable compilation.
- Support for training on multiple GPUs with the help of the
  [`accelerate`](https://huggingface.co/docs/accelerate/index) package has been
  improved by fixing some bugs and providing a dedicated [history
  class](https://skorch.readthedocs.io/en/latest/user/history.html#distributed-history).
  Our documentation contains more information on [what to consider when training
  on multiple
  GPUs](https://skorch.readthedocs.io/en/latest/user/huggingface.html#caution-when-using-a-multi-gpu-setup).
- If you have ever been frustrated with your neural net not training properly,
  you know how hard it can be to discover the underlying issue. Using the new
  [`SkorchDoctor`](https://skorch.readthedocs.io/en/latest/helper.html#skorch.helper.SkorchDoctor)
  class will simplify the diagnosis of underlying issues. Take a look at the
  accompanying
  [notebook](https://nbviewer.org/github/skorch-dev/skorch/blob/master/notebooks/Skorch_Doctor.ipynb)

Apart from that, a few bugs have been fixed and the included notebooks have been
updated to properly install requirements on Google Colab.

We are grateful for external contributors, many thanks to:

- Kshiteej K (kshitij12345)
- Muhammad Abdullah (abdulasiraj)
- Royi (RoyiAvital)
- Sawradip Saha (sawradip)
- y10ab1 (y10ab1)

Find below the list of all changes since v0.12.1 below:

### Added
- Add support for compiled PyTorch modules using the `torch.compile` function,
  introduced in [PyTorch 2.0
  release](https://pytorch.org/get-started/pytorch-2.0/), which can greatly
  improve performance on new GPU architectures; to use it, initialize your net
  with the `compile=True` argument, further compilation arguments can be
  specified using the dunder notation, e.g. `compile__dynamic=True`
- Add a class
  [`DistributedHistory`](https://skorch.readthedocs.io/en/latest/history.html#skorch.history.DistributedHistory)
  which should be used when training in a multi GPU setting (#955)
- `SkorchDoctor`: A helper class that assists in understanding and debugging the
  neural net training, see [this
  notebook](https://nbviewer.org/github/skorch-dev/skorch/blob/master/notebooks/Skorch_Doctor.ipynb)
  (#912)
- When using `AccelerateMixin`, it is now possible to prevent unwrapping of the
  modules by setting `unwrap_after_train=True` (#963)

### Fixed
- Fixed install command to work with recent changes in Google Colab (#928)
- Fixed a couple of bugs related to using non-default modules and criteria
  (#927)
- Fixed a bug when using `AccelerateMixin` in a multi-GPU setup (#947)
- `_get_param_names` returns a list instead of a generator so that subsequent
  error messages return useful information instead of a generator `repr` string
  (#925)
- Fixed a bug that caused modules to not be sufficiently unwrapped at the end of
  training when using `AccelerateMixin`, which could prevent them from being
  pickleable (#963)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants