CUQI-DTU / CUQIpy

https://cuqi-dtu.github.io/CUQIpy/
Apache License 2.0
48 stars 9 forks source link

Consider getting rid of the _pre_warmup and _pre_sample methods in NUTS #537

Closed amal-ghamdi closed 1 month ago

amal-ghamdi commented 2 months ago

Description:

consider getting rid of the _pre_warmup and _pre_sample methods in NUTS. See the discussion in https://github.com/CUQI-DTU/CUQIpy/pull/408. This will also lead to updating the new Sampler class and HybridGibbs to remove special treatment for samplers that have pre_sample and pre_warmup

DoD:

amal-ghamdi commented 1 month ago

Can you review the DoD, @jakobsj ? Thank you.

jakobsj commented 1 month ago

@amal-ghamdi any unit tests to modify or remove? If not then approved

amal-ghamdi commented 1 month ago

Thank you @jakobsj , Yes good point, I added a test for NUTS within Gibbs in DoD.

A related point, I also investigated the already-existing regression tests (from when we made the new NUTS in the experimental module) to make sure chains are actually identical (between old and new NUTS), and it was. So I think it is robust in terms of matching the old NUTS.