Remove asserts on loc and scale in distributions that do not support them #734
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mbohlkeschneider
approved these changes
Mar 30, 2020
There was a problem hiding this comment.
I think the general problem here, is that we somehow need to assume that the user/developer understands the underpinnings of the network/distribution interplay here. I think in the long run, it is better to have standardized blocks for networks that compute the loc/scale from past data and pass them as a composed TransformedDistribution.
lfywork
added a commit
to lfywork/gluon-ts
that referenced
this pull request
Apr 3, 2020
remove asserts on loc and scale (awslabs#734)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available: fixes #641
Description of changes: For some distributions, it doesn't make sense to prescribe a "loc" or "scale" by which to translate and scale the distribution arguments. However, networks that do compute a location/scale for the (past) data (and potentially use them to scale the network inputs) may provide this information to the DistributionOutput class when invoking the
distributionmethod: in these cases, we currently raise an exception. This is not ideal, because it means that you cannot excange a distribution family that does support loc/scale for one that doesn't, on top of the same network. I think the best solution is to simply ignore loc and scale in DistributionOutput classes that don't support them.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.