search

desihub / desitarget

DESI Targeting
BSD 3-Clause "New" or "Revised" License
18 stars 23 forks source link

Implement proper rounding of pixel coordinates for randoms #782

Closed rongpu closed 2 years ago

rongpu commented 2 years ago

This PR fixes a bug in randoms.py that inaccurately rounds the pixel coordates. Specifically, astype("int") rounds towards zero, but it should be rounded to the nearest integer.

geordie666 commented 2 years ago

Thanks for catching this @rongpu. Your changes look correct to me.

Can you please update "No changes yet" in the changes.rst file to describe this PR? I'll then merge this.

I'm working on a separate PR related to the randoms, which I should finish this week. Once I'm done with that, I guess I should make new versions of the randoms.

rongpu commented 2 years ago

Thanks @geordie666 . I have updated the changes.rst file. Please feel free to make any edits.

As for producing new randoms, there is a separate issue with the WISE depth discrepancy, and I guess we might want to wait a bit in case we make any decisions/changes on that issue soon.

geordie666 commented 2 years ago

Yes, good point about waiting on the WISE-depth-issue. I guess the current randoms are "good enough" for most analyses, so we may as well wait.

I'll merge this PR once tests pass.

geordie666 commented 2 years ago

I think the unit test failure is unrelated to the updates in this branch, but I can't easily work on a fix until Cori is back. So, I'll merge this tomorrow morning and look at the unit tests then.