search

sbi-dev / sbi

Simulation-based inference toolkit
https://sbi-dev.github.io/sbi/
Apache License 2.0
590 stars 152 forks source link

add test for passing weights to ensemble posterior #1304

Open janfb opened 4 days ago

janfb commented 4 days ago

There was a bug in the type checking for the weights passed to the ensemble posterior, see #1299

This was not detected before because we are missing a test case for this.

To fix this issues, add a minimal test similar to here: https://github.com/sbi-dev/sbi/blob/18b114190820a7a460fe3c106ebaedb8ac1f0bbb/tests/ensemble_test.py#L57

e.g., by passing uniform weights as list or as tensors (both cases should be covered. Ideally, we do not run the entire again, but just a minimal version that tests the API: minimal training, only two posteriors etc.

samadpls commented 2 days ago

Hi @janfb, can I work on this issue?

janfb commented 2 days ago

Hi @samadpls , yes please do. do you have a plan how to tackle this, or do you need more input?

samadpls commented 2 days ago

I'll follow the steps you mentioned above to add a targeted test for weights in EnsemblePosterior, verifying compatibility for both lists and tensors. I'd appreciate any input you might have on additional cases to include

janfb commented 2 days ago

Sure. the current test tests for the accuracy of the posteriors as well, and is therefore marked as slow.

Thinking about it, it would make sense to leave this one as it is, and to create a new test that includes the cases with different types of weights, but that does not check for posterior accuracy. It should only test the API and be rather fast (please test this), and therefore would not need the @slow mark.

Thanks!