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

Numerical error in HealpixNestedPolygonComputer.overlappingCells #18

Closed
mbtaylor opened this issue Apr 5, 2024 · 5 comments
Closed

Comments

@mbtaylor
Copy link
Contributor

mbtaylor commented Apr 5, 2024

Hi @fxpineau. I see a problem if I run the following code against cds-healpix-java v0.30.2:

import cds.healpix.Healpix;
import cds.healpix.HealpixNestedPolygonComputer;

public class Overlap {
    public static void main(String[] args) {
        int order = 12;
        HealpixNestedPolygonComputer pc = Healpix.getNested( 12 ).newPolygonComputer();
        double[][] verts1 = new double[][] {
            { -1.5702949547333407, -0.7295093151415473 },
            { -1.5702171673769950, -0.7295093016804524 },
            { -1.5701393800214274, -0.7295092852142693 },
            { -1.5700615926667945, -0.7295092657429985 },
        };
        double[][] verts2 = new double[][] {
            { -1.5706045044233712, -0.7295105218483977 },
            { -1.5705168372776197, -0.7295105199399403 },
            { -1.5704291701320918, -0.7295105142145686 },
            { -1.5703415029870114, -0.7295105046722821 },
        };
        pc.overlappingCells( verts1 );  // OK
        pc.overlappingCells( verts2 );  // trouble
    }
}

The calculation with the second set of vertices gives me an AssertionError at cds.healpix.NewtonMethod.arcSpecialPointInEqr(NewtonMethod.java:234), or a NullPointerException later on if assertions are turned off. This is probably some kind of edge case since this code has been in topcat for several years and I haven't noticed it before now.

@fxpineau
Copy link
Member

fxpineau commented Apr 8, 2024

Hi @mbtaylor ,

Thank you for the report.

The good news is that the rust library seems to be working well for both polygons.
See the screenshot made in Aladin:
Poly1and2

I am going to check the differences between both libraries to fix the Java one (today or tomorrow).

fx

fxpineau added a commit that referenced this issue Apr 9, 2024
@fxpineau
Copy link
Member

fxpineau commented Apr 9, 2024

Fixed in commit b688015 (if the Newton method fails, we are in a corner case
which probably does not contains "special points"; if we found a corner case having a special point we could fallback with a dichotomic appoach).

@fxpineau fxpineau closed this as completed Apr 9, 2024
@mbtaylor
Copy link
Contributor Author

mbtaylor commented Apr 9, 2024

Thanks @fxpineau, I confirm that fixes my test case and the application issue I encountered that led to the report.
Will you assign a new release number/tag to this version?

@fxpineau
Copy link
Member

fxpineau commented Apr 9, 2024

Sure. I just updated the CHANGES file, the version number and added a tag.
Thank you @mbtaylor .

@mbtaylor
Copy link
Contributor Author

mbtaylor commented Apr 9, 2024

Thanks!

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