Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
There was a problem hiding this comment.
Pull request overview
This PR lays groundwork for integrating BayesFlow likelihood-ratio estimators (LRE) into the HSSM workflow by adding an installable optional dependency set and exposing the new tutorial in the docs, alongside a small change to how HSSM.sample() forwards the chosen inference engine to Bambi.
Changes:
- Forward
samplerdirectly asinference_methodintobmb.Model.fit()fromHSSM.sample(). - Add a
bayesflowoptional dependency extra (bayesflow,keras) to support the integration tutorial. - Register the new BayesFlow LRE tutorial notebook in the MkDocs navigation and (non-)execution configuration.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/hssm/hssm.py |
Adjusts how the selected sampler/inference engine is passed into Bambi’s fit() call. |
pyproject.toml |
Adds an optional dependency extra to enable pip install hssm[bayesflow]. |
mkdocs.yml |
Adds the BayesFlow LRE tutorial to the docs nav and mkdocs-jupyter config. |
Comments suppressed due to low confidence (1)
src/hssm/hssm.py:794
hssm.HSSM.sample()now passessamplerstraight through asinference_methodtobmb.Model.fit(). There are existing tests that callsample(sampler="numpyro"), but they only assert that anInferenceDatais returned, so they won’t catch regressions where the requested sampler is ignored or mapped incorrectly. Consider adding a regression check that the requested sampler/inference method is actually used (e.g., assert on metadata recorded in the returnedInferenceData, or mock/spy onbmb.Model.fitto verify the argument value).
self._inference_obj = self.model.fit(
inference_method=sampler,
init=init,
include_response_params=include_response_params,
omit_offsets=omit_offsets,
**kwargs,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@digicosmos86 could you give a quick glance at the sampler routing logic and approve / request changes if you disagree with anything there? Trying to get it into the release. |
digicosmos86
left a comment
There was a problem hiding this comment.
LGTM! Seems like an easy integration with HSSM through a callable
This is the first of two PRs and should serve as the basis for a decision on what kind of glue-code to own on this front.
In principle, we should be able to incorporate these types of networks natively as an option / addition into our overall pipeline.
Small bugfix along the way, bug was discovered while testing.
This pull request introduces integration with BayesFlow LRE and updates documentation and dependencies accordingly. The most important changes are grouped below by theme.
BayesFlow LRE Integration:
bayesflowinpyproject.tomlto support BayesFlow integration, including required packagesbayesflowandkeras.tutorials/bayesflow_lre_integration.ipynbis now included in the documentation navigation and build process. [1] [2]Inference Method Handling:
samplemethod ofsrc/hssm/hssm.pyby directly passing thesamplerargument, allowing for more flexible integration of new inference engines like BayesFlow.