Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Condition for drawing additional samples - n_eff #37

Open
NolanSmyth opened this issue Mar 3, 2025 · 1 comment
Open

Condition for drawing additional samples - n_eff #37

NolanSmyth opened this issue Mar 3, 2025 · 1 comment

Comments

@NolanSmyth
Copy link

I'm wondering if there is an error in the conditional check on line 900 of engine.py in the predict function? Currently, the code checks:

if n_max > n_samples and neff > neff_min:

However, I would think the intended logic should be to trigger additional simulations only when the effective sample size is below the minimum threshold. In that case, the condition is:

if n_max > n_samples and neff < neff_min:

In a separate but closely related issue - it would be nice to ensure that there are n_samples returned for the event of no importance sampling. I.e. in line 880:

ys = self._draw_params(x, n_samples)

one could generate more samples in the event that samples outside the prior are removed in line 1245:

params = params[~np.isinf(logprior)].

For example, a hacky way of doing this is to replace line 880 with:

n_total = 0
ys = []
while n_total < n_samples:
    n_needed = n_samples - n_total
    new_ys = self._draw_params(x, int(n_needed*1.1))
    ys.append(new_ys)
    n_total += len(new_ys)

ys = np.concatenate(ys)[:n_samples]

Thanks a lot

kmzzhang added a commit that referenced this issue Mar 8, 2025
@kmzzhang
Copy link
Owner

kmzzhang commented Mar 8, 2025

Good catch, thanks! Both fixed.
Replacing out of prior samples fixed in engine._draw_params, since SNPE will need this too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants