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

Add general coupling transformation #23

Merged
merged 6 commits into from
Jul 13, 2023

Conversation

simonschnake
Copy link
Contributor

@simonschnake simonschnake commented Jul 11, 2023

Adds

  • Transform: CouplingTransform
  • TransformModule: MaskedCouplingTransform
  • FlowModule: CouplingFlow

Details discussed in the discussion #22

Simon Schnake and others added 3 commits July 11, 2023 11:38
* Rename to GeneralCouplingTransform to match Dinh et al. (2014)
* Use new unpack helper
* Use checkered coupling masks by default
* Use more concise notations in CouplingTransform
* Fix GCF's __init__ and extra_repr
@francois-rozet
Copy link
Member

francois-rozet commented Jul 13, 2023

Hello @simonschnake, I have made a few changes to the PR. It is a major feature addition so it required some care, notably for the tests. I have settled for the name GeneralCouplingTransform as written by Dinh et al. (2014). I also fixed a few issues with the CouplingFlow (renamed GCF for consistency). Most of the architecture and docstrings remains the same, though, so good job! I will do some more tests, but I think it is ready to merge otherwise 🔥

@francois-rozet francois-rozet changed the title CouplingFlow Add general coupling transformation Jul 13, 2023
@francois-rozet francois-rozet merged commit 85a0ff9 into probabilists:master Jul 13, 2023
@simonschnake simonschnake deleted the CouplingFlow branch July 14, 2023 11:21
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.

2 participants