Refactor YehHummer to use linear (D_0, slope) parameterization#206
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the YehHummer finite-size correction fitting from (D_0, viscosity) parameterization to (D_0, slope) parameterization to improve MCMC sampling efficiency. The linear relationship D_PBC = D_0 - slope/L is more suitable for sampling than the original formulation where viscosity spans many orders of magnitude. Additionally, validation logic is added to the base fitting class to handle invalid model outputs.
Changes:
- Refactored YehHummer to fit slope directly instead of viscosity, with automatic conversion to viscosity for output via the shear_viscosity property
- Inverted viscosity bounds to slope bounds (higher viscosity = lower slope)
- Simplified the fitting function to use the inline linear formula D_PBC = D_0 - slope/L
- Added validation in FittingBase.log_likelihood to reject non-finite or non-positive model values
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| kinisi/yeh_hummer.py | Refactored from (D_0, viscosity) to (D_0, slope) parameterization; removed static method; updated shear_viscosity property to convert slope samples |
| kinisi/fitting.py | Added validation for invalid model values in log_likelihood method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
arm61
left a comment
arm61
left a comment
There was a problem hiding this comment.
This is a nice change. A few things to double-check/change, then it can be merged.
kinisi/fitting.py
Outdated
| x_values = self.get_independent_variable() | ||
| model = self.function(x_values, *parameters) | ||
|
|
||
| # Handle invalid model values (e.g., from VTF when T0 >= T) |
There was a problem hiding this comment.
This should be handled with an appropriate prior probability.
kinisi/yeh_hummer.py
Outdated
| :param box_lengths: Array of box lengths / Angstrom | ||
| :param D_0: Infinite-system diffusion coefficient | ||
| :param viscosity: Shear viscosity | ||
| :param slope: Slope = (k_B * T * xi) / (6 * pi * eta) |
There was a problem hiding this comment.
| :param slope: Slope = (k_B * T * xi) / (6 * pi * eta) | |
| :param slope: i.e., (k_B * T * xi) / (6 * pi * eta) |
Reparameterize YehHummer fitting from (D_0, viscosity) to (D_0, slope) where the model is linear: D_PBC = D_0 - slope/L. This improves MCMC sampling efficiency since viscosity spans many orders of magnitude while slope is better behaved in parameter space.
Changes to yeh_hummer.py:
Changes to fitting.py:
-inffor non-finite or non-positive model outputs(e.g., VTF when T0 >= T)