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

Added 32-bit CircleCI configuration #78

Merged
merged 4 commits into from
Nov 25, 2016
Merged

Conversation

astrofrog
Copy link
Member

Since we have Cython geometrical code, we should test this!

@astrofrog
Copy link
Member Author

(I should rebase this once #71 is merged so that we can check if there are any issues with the Cython code)

@bsipocz
Copy link
Member

bsipocz commented Nov 25, 2016

@astrofrog - do you think we should do this for photutils, too?

@astrofrog
Copy link
Member Author

astrofrog commented Nov 25, 2016

@bsipocz - yes, though once there is a new version of regions out, we can immediately switch over photutils to use the Cython functions here to avoid duplication (even before we refactor photutils to use the region infrastructure)

@astrofrog
Copy link
Member Author

Turns out the 32-bit testing has already been useful to identify some corner cases! (feff1be)

@astrofrog
Copy link
Member Author

This should NOT be merged until #71 is merged, after which this should be rebased.

"""
In some of the geometrical functions, we have to take the sqrt of a number
and we know that the number should be >= 0. However, in some cases the
value is e.g. -1e-10, but we want to treat it as zero, which is what
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we maybe make sure it's indeed close to zero and if not failing the code is OK.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is that then we have to then choose a threshold which will be architecture dependent. I would prefer to leave this as a function that we should call only if we are sure the result should be >= 0 modulo numerical uncertainty.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just added a note about this though

@astrofrog astrofrog merged commit 8db809e into astropy:master Nov 25, 2016
@cdeil cdeil added this to the 0.2 milestone Jun 17, 2019
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

Successfully merging this pull request may close these issues.

None yet

3 participants