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

Possible bug in l1.py/apportion_counts_to_resultants #117

Closed
MichaelDAlbrow opened this issue May 1, 2024 · 3 comments
Closed

Possible bug in l1.py/apportion_counts_to_resultants #117

MichaelDAlbrow opened this issue May 1, 2024 · 3 comments

Comments

@MichaelDAlbrow
Copy link

Line 291 in l1.py:

read = rng_numpy.binomial((counts - counts_so_far).astype('i4'), p)

When generating an image with a large number of stars (from an external catalog) I occasionally get the condition
counts - counts_so_far being less than zero, which causes a crash.

A workaround that at least allows the code to continue is to replace this line with:

        n = (counts - counts_so_far).astype('i4')
        n[n < 0] = 0
        read = rng_numpy.binomial(n, p)

Michael Albrow

@schlafly
Copy link
Collaborator

schlafly commented May 2, 2024

Thanks Michael. Are these stars very bright? I guess in retrospect I can imagine that since the galsim image is floats, then I cast to an integer, and that leads counts_so_far to be too large, and then this goes negative, maybe? It feels like that cast can only go wrong for very large floats, though, where we're talking about injecting about injecting millions of counts.

Obviously your suggestion will resolve the issue, but I'd like to make sure that I understand whether it's masking some other underlying problem or if this is "just" floating point roundoff.

There also must be issues when the number of photons being simulated doesn't cast well to an i4. Maybe the most straightforward approach if that ends up being the issue would be to replace counts -> np.clip(counts, 2**31 - 1).astype('i4') at the beginning.

@MichaelDAlbrow
Copy link
Author

This was a very crowded simulation, with many saturated stars, so I expect that your guess is correct.

Michael

@schlafly
Copy link
Collaborator

schlafly commented May 3, 2024

I tried to make this more robust here. #119 If that doesn't address your case, it would be helpful to have an example I can reproduce.

@schlafly schlafly closed this as completed May 3, 2024
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