search

guilgautier / DPPy

Python toolbox for sampling Determinantal Point Processes
https://dppy.readthedocs.io
MIT License
219 stars 52 forks source link

Lost in rejection #72

Closed rbardenet closed 7 months ago

rbardenet commented 7 months ago

Hi @guilgautier,

we were reading the code of multivariate_jacobi_ope.MultivariateJacobiOPE.sample() with Thibaut, and I was puzzled by not seeing the rejection bound $N/(N-n+1)$ at line 444, when we check the rejection condition. In the documentation, we do show the rejection bound, but it's not explicitly there in the code. Is it implicitly computed in the Schur complement?

All the best, Rémi

guilgautier commented 7 months ago

Hi @rbardenet and Thibaut!

It turns out that the rejection bound cancels out with the ratio of the normalizing constants of the proposal (marginal) and the target (conditional), cf https://github.com/guilgautier/DPPy/blob/v1-dev/dppy/continuous/abstract_continuous_dpp.py#L246-L249 Let me know if that's misleading, otherwise you can safely close the issue.

More generally, I suggest to work with this v1-dev version, where things are better structured, but still unfinished 🙄

rbardenet commented 7 months ago

You are right, the normalization constants cancel out, silly me! In the v1-dev, I would maybe call the objects unnormalized_target_x and unnormalized_proposal_x, or maybe just comment that the normalization constants cancel out with the bound, to avoid myself getting confused five years from now :)