shred / commons-suncalc

Calculate sun and moon times in Java
Apache License 2.0
125 stars 22 forks source link

Moon phase value does not reach +/-180 degrees #44

Closed m4tiz5zktonj closed 8 months ago

m4tiz5zktonj commented 8 months ago

Hello.

I'm using this library to draw the Moon phase based on the org.shredzone.commons.suncalc.MoonIllumination#getPhase value and I'm having a visual glitch around New Moon times. This happens because of returned value, that does not reach -180 and +180, and boundary values are different each lunar cycle.

The following test demonstrates the problem:

@Test
public void testBadPhaseValue() {
    MoonIllumination mi = MoonIllumination.compute()
                    .on(2017, 6, 24, 2, 44, 28)
                    .utc()
                    .execute();
    assertThat(mi.getPhase()).as("phase").isCloseTo(175.94304962023995, Offset.offset(1e-15));
    mi = MoonIllumination.compute()
                    .on(2017, 6, 24, 2, 44, 29)
                    .utc()
                    .execute();
    assertThat(mi.getPhase()).as("phase").isCloseTo(-175.94305557604852, Offset.offset(1e-15));

    mi = MoonIllumination.compute()
                    .on(2024, 1, 11, 10, 39, 1)
                    .utc()
                    .execute();
    assertThat(mi.getPhase()).as("phase").isCloseTo(174.94013705599542, Offset.offset(1e-15));
    mi = MoonIllumination.compute()
                    .on(2024, 1, 11, 10, 39, 2)
                    .utc()
                    .execute();
    assertThat(mi.getPhase()).as("phase").isCloseTo(-174.94016088988388, Offset.offset(1e-15));

    mi = MoonIllumination.compute()
                    .on(2024, 2, 9, 20, 34, 40)
                    .utc()
                    .execute();
    assertThat(mi.getPhase()).as("phase").isCloseTo(175.44333529421183, Offset.offset(1e-15));
    mi = MoonIllumination.compute()
                    .on(2024, 2, 9, 20, 34, 41)
                    .utc()
                    .execute();
    assertThat(mi.getPhase()).as("phase").isCloseTo(-175.4433949717004, Offset.offset(1e-15));
}

Boundary values are different: +-175.9, +-174.9, +-175.4, so the real value range cannot be simply scaled to -180..180.

Could you, please, fix this or give some advice how to bypass this?

Thanks.

shred commented 8 months ago

It looks like these are rounding errors from the calculation. I don't think there is a way to fix it.

I would propose a different approach. You could calculate the Duration between two subsequent new moons, and also the Duration between the first new moon and the current time. You could then use the rule of three to calculate a "visual moon phase". This would be free of visual glitches.

m4tiz5zktonj commented 8 months ago

Are you suggesting to use MoonPhase class for searching New Moon? I'm afraid I can't do this.

Sorry, I was not correct, when I said "I'm using this library". I use code that I port to a different language that is meant to be run on platform with VERY low computing power. I want to display Moon rise, set, azimuth/elevation, illimination value and a graphical representation of phase. I've already reduced number of calculations while mostly keeping the same accuracy in MoonTimes to have barely acceptable performance while scanning 24 hours for Moon rise and set. If I add MoonPhase with moonphase scanning, then my solution will definitely exceed platform limits.

Can't the org.shredzone.commons.suncalc.MoonIllumination#getFraction somehow be adjusted and used as phase? It got pretty close to 0 and 1, but I need somehow determine whether it is first or last quarter, etc.

shred commented 8 months ago

Can't you use getFraction() and getPhase() together, to find out the phase and the quarter?

m4tiz5zktonj commented 8 months ago

Yes, probably, I'll add something like correctedPhase with 0..1 range which value is based on fraction and is adjusted by phase.

Thank you very much!

shred commented 8 months ago

You're welcome. I will close this issue, but feel free to reopen it if necessary.