-
-
Notifications
You must be signed in to change notification settings - Fork 18
feat: improvements to normalizing flow #196
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
base: main
Are you sure you want to change the base?
Conversation
be60be1
to
632dcba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR aims to improve the normalizing flow routines by repairing householder transformation layers, modifying the asymmetric transformation, and adding additional flow options for experimentation.
- Update to test configuration for Stan and PyMC models
- Enhancements to the transform adapter with new parameters and adjusted training thresholds
- Updates to project configuration files including Cargo.toml and CI workflows
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tests/test_stan.py | Adds jax configuration management and test adjustments |
tests/test_pymc.py | Simplifies normalization flow tests and reduces the number of draws |
tests/reference/*.txt | Updates reference output files for deterministic sampling |
python/nutpie/transform_adapter.py | Updates loss threshold, batch size, optimizer, and flow parameters |
pyproject.toml | Updates Python requirement and adds a dependency for pytest-arraydiff |
docs/.gitignore | Adds .quarto/ to the ignored files |
Cargo.toml | Bumps tch version and increases opt-level |
.github/workflows/ci.yml | Adds pytest-arraydiff flag to the CI pipeline |
.gitattributes | Adds a highlighting configuration for SCM files |
Comments suppressed due to low confidence (3)
python/nutpie/transform_adapter.py:682
- Ensure that changing the loss threshold from -5 to -4 is intentional, as this lower threshold may affect training termination behavior.
if np.isfinite(old_loss) and old_loss < -4 and self.index > 10:
tests/test_pymc.py:292
- [nitpick] Reducing the number of draws from 2000 to 500 may impact the statistical robustness of the test; please confirm that 500 draws are sufficient for your reproducibility requirements.
draws=500,
Cargo.toml:53
- [nitpick] Increasing opt-level to 3 can affect compile times and binary size; verify that this change aligns with production performance goals.
opt-level = 3
@@ -44,14 +43,15 @@ def fit_to_data( | |||
loss_fn: Callable | None = None, | |||
max_epochs: int = 100, | |||
max_patience: int = 5, | |||
batch_size: int = 100, | |||
batch_size: int = 128, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The default batch size was updated from 100 to 128; consider updating any related documentation or comments to reflect this change.
Copilot uses AI. Check for mistakes.
This repairs the householder transformation layers, changes the asymmetric transformation a bit and brings a couple more normalizing flow options for experimentation.
@jessegrabowski Maybe you want to check if this repairs your model with householder transforms...