[RFC] Feature facilitate customization of net

[BenjaminBossan]

The motivation for this PR is to make skorch easier to customize so that things like DCGAN can be implemented with less effort. See the attached notebook for an illustration of this.

To achieve this, I introduced 2 major changes:

PassthroughScoring

Currently, when adding new losses on a batch level, it's very fiddly to log them on an epoch level. You have to define a custom function that picks the right value from history and passes it through without modification, then add a BatchScoring callback that uses this mock scoring function, defines the name of the score again, and pass noop.

Here is the code before and after:

# somewhere in the net code
    ...
    self.history.record_batch('my_loss', my_loss)
    ...

# before
from skorch.callbacks import BatchScoring
from skorch.utils import noop

def my_loss_score(net, X=None, y=None):
    return net.history[-1, 'batches', -1, 'my_loss']

callbacks = [
    BatchScoring(
        my_loss_score,
        name='my_loss',
        target_extractor=noop,
    ),
]

# after
from skorch.callbacks import PassthroughScoring

callbacks = [
    PassthroughScoring(name='my_loss'),
]

I replaced BatchScoring with PassthroughScoring in the existing code for train and valid loss.

Facilitate adding custom torch modules and optimizers

Right now, if a user wants to add their own modules or optimizers to NeuralNet, there are a few non-obvious steps they need to take for everything to work well:

1. Make sure to extend prefixes_ attribute to make set_params work (but don't mutate or else woe!)
2. Make sure to extend cuda_dependent_attributes_ to avoid subtle bugs (but again, don't mutate!)
3. Use "private" methods like self._get_params_for or self._get_params_for_optimizer.

The changes introduces in this PR automatically take care of 1. and 2. by using some __setattr__ magic. This is of course a dangerous thing to do, so please think about cases where this could go wrong.

For 3., I have not found a better solution right now then to make the mentioned methods public. I was experimenting with automating this step completely with some magic things happening behind the door but this was a) a bit hacky and b) we cannot know which module parameters should be passed to an optimizer, i.e. we cannot fully automate this.

If anyone has a suggestion to make this easier for the user, please come ahead.

Conclusion

To illustrate how all these changes help, please have a look at the attached notebook. This replaces the notebook shown in #587 and sticks much closer to the original pytorch implementation of DCGAN.

If we find this solution okay, I would add more documentation, docstrings, and tests.

ping @zachbellay @cgarciae @YannDubs

[review-notebook-app]

Check out this pull request on 

You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB.

[BenjaminBossan]

After discussion with @ottonemo, it might be better to split the introduction of PassthroughScoring from the changes relating to __setattr__. I'll leave this here for now, but keep this option in mind.

[ottonemo]

The changes introduces in this PR automatically take care of 1. and 2. by using some setattr magic. This is of course a dangerous thing to do, so please think about cases where this could go wrong.

I think using __setattr__ is fine and actually in line with how PyTorch modules register their subordinate parameters. We probably should handle __delattr__ as well, even if there are no references (yet) added, there might be in the future and then this will bite us in unexpected ways.

For 3., I have not found a better solution right now then to make the mentioned methods public. I was experimenting with automating this step completely with some magic things happening behind the door but this was a) a bit hacky and b) we cannot know which module parameters should be passed to an optimizer, i.e. we cannot fully automate this.

I'm for making this step explicit and to just expose the methods publicly. They are side-effect free and therefore there is no harm done in doing so.

[BenjaminBossan]

We probably should handle __delattr__ as well

Good point, I'll look into it. As you said, for now we'd probably be fine.