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

Adding 1-dimensional version of RationalQuadraticSpline and tests for conditional blocks #151

Merged
merged 5 commits into from
Mar 3, 2023

Conversation

RussellALA
Copy link
Collaborator

  • Added new InvertibleModule RationalQuadraticSplines_1D that can be used as invertible learnable transform when couplings are not desirable (e.g. when the feature dimension is 1).
  • Restructured the BinnedSpline class s.t. it works as an InvertibleModule instead of a Coupling
  • Added BinnedSplineCoupling class to replace the old BinnedSpline
  • Added conditional SequenceINN tests in spline_tests.py

@@ -44,6 +45,65 @@ def _spline2(self, x: torch.Tensor, parameters: Dict[str, torch.Tensor], rev: bo
return rational_quadratic_spline(x, left, right, bottom, top, deltas_left, deltas_right, rev=rev)


class RationalQuadraticSpline_1D(BinnedSpline):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the name might be misleading… 1D suggests that one only can have 1D transformations, but this is not the case

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Renamed to ElementwiseRationalQuadraticSpline

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good to me!

…ine. Renamed BinnedSplineCoupling to BinnedSpline for backwards compatability. Renamed existing BinnedSpline to BinnedSplineBase
@fdraxler fdraxler requested a review from LarsKue March 2, 2023 12:59
Copy link
Collaborator

@LarsKue LarsKue left a comment

Choose a reason for hiding this comment

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

Code looks good. I like the restructured BinnedSpline. Might be worth looking into refactoring the overall structure of InvertibleModule vs Coupling in general (including the file structure). But I think we should relay that to the future and merge this as-is.

@RussellALA RussellALA merged commit ebba9d2 into vislearn:master Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants