hablullah / go-prayer

Go package for getting salat times in a specific location
MIT License
60 stars 8 forks source link

Crash: panic: Cannot create a Decimal from NaN #2

Closed CamilleScholtz closed 4 years ago

CamilleScholtz commented 4 years ago

Error log:

panic: Cannot create a Decimal from NaN

goroutine 25 [running]:
github.com/shopspring/decimal.newFromFloat(0x7ff8000000000001, 0x7ff8000000000001, 0xbd1bf0, 0xbff0626d4dcc0f2e, 0x7ff8000000000001)
    /home/onodera/wrk/pkg/mod/github.com/shopspring/decimal@v0.0.0-20180709203117-cd690d0c9e24/decimal.go:210 +0x443
github.com/shopspring/decimal.NewFromFloat(0x7ff8000000000001, 0x7ff8000000000001, 0xbff0626d4dcc0f2e)
    /home/onodera/wrk/pkg/mod/github.com/shopspring/decimal@v0.0.0-20180709203117-cd690d0c9e24/decimal.go:186 +0x92
github.com/RadhiFadlillah/go-prayer/internal/trigonometry.Acos(0xc0010fa980, 0xc0fffffff0, 0xc0010fa8c0, 0xffffffe0)
    /home/onodera/wrk/pkg/mod/github.com/!radhi!fadlillah/go-prayer@v0.0.0-20190915074711-34100aa56d18/internal/trigonometry/trigonometry.go:35 +0x57
github.com/RadhiFadlillah/go-prayer/internal/prayer.TimeCalculator.getHourAngle(0x4035000000000000, 0xc0010c01c0, 0xfffffffc, 0xc0010c01e0, 0xfffffffc, 0xc0010c0160, 0x0, 0xc0010c0180, 0x0, 0xc0010c01a0, ...)
    /home/onodera/wrk/pkg/mod/github.com/!radhi!fadlillah/go-prayer@v0.0.0-20190915074711-34100aa56d18/internal/prayer/calculator.go:137 +0x1da
github.com/RadhiFadlillah/go-prayer/internal/prayer.TimeCalculator.GetFajrTime(0x4035000000000000, 0xc0010c01c0, 0xfffffffc, 0xc0010c01e0, 0xfffffffc, 0xc0010c0160, 0x0, 0xc0010c0180, 0x0, 0xc0010c01a0, ...)
    /home/onodera/wrk/pkg/mod/github.com/!radhi!fadlillah/go-prayer@v0.0.0-20190915074711-34100aa56d18/internal/prayer/calculator.go:59 +0x86
github.com/RadhiFadlillah/go-prayer.GetTimes(0xbfaa3a7d46adac71, 0x1f358fb3, 0xc16440, 0x404a1058793dd97f, 0x4016aca57a786c22, 0x4035000000000000, 0x0, 0x0, 0x1, 0x0, ...)
    /home/onodera/wrk/pkg/mod/github.com/!radhi!fadlillah/go-prayer@v0.0.0-20190915074711-34100aa56d18/calculator.go:74 +0x2bb
main.(*Bar).initPopups.func1()
    /home/onodera/wrk/src/github.com/onodera-punpun/melonbar/popups.go:40 +0x1fd
main.(*Bar).drawPopup(0xc000ce0460, 0x7cf0bf, 0x5, 0x4, 0x74c760)
    /home/onodera/wrk/src/github.com/onodera-punpun/melonbar/popup.go:75 +0x334
main.(*Bar).initBlocks.func17(0xc0005f2322, 0xc000116910)
    /home/onodera/wrk/src/github.com/onodera-punpun/melonbar/blocks.go:293 +0x3f
created by main.(*Bar).drawBlocks.func1
    /home/onodera/wrk/src/github.com/onodera-punpun/melonbar/block.go:103 +0x23a

I have a feeling this is because something rounds to zero or something, which makes the decimal package return a panic.

Code that crashes (for me):

            // Get the prayer times.
            ptl, _ := prayer.GetTimes(time.Now(), prayer.Config{
                Latitude:          52.1277,
                Longitude:         5.6686,
                Elevation:         21,
                CalculationMethod: prayer.MWL,
                AsrJuristicMethod: prayer.Hanafi,
                PreciseToSeconds:  false,
            })

Slightly changing the Latitude to 51 for example fixes the crash, also the crash didn't happen last week.

CamilleScholtz commented 4 years ago

It seems like cosValue in the Acos function in trigonomity.go has a value of -1.0705466758213495, the golang manpage about math.Acos says:

Special condition:

Acos(x) = NaN if x < -1 or x > 1
CamilleScholtz commented 4 years ago

Fixed by adding math.Max(fl, -1) to the Acos function, though I have no idea if this throws the calculations off.

CamilleScholtz commented 4 years ago

Is this package still being developed? Else I will switch over to an alternative.

RadhiFadlillah commented 4 years ago

Sorry for late response.

I've added a check to make sure the cos value only between -1 and 1. Fortunately it seems the calculations is still quite accurate, at least when compared with SalahTimes.

RadhiFadlillah commented 4 years ago

Sorry. I recheck your error report above and it seems the panic is happened when calculating Fajr time.

After further read, I found out that cos value of hour angle is < -1 for area with high latitude where sun never set. However, your latitude is not high enough to trigger this panic.

With that said, my fix above is only heal the symptom and not the root cause. I'll keep this issue open until I found out how to fix it.

RadhiFadlillah commented 4 years ago

Hi @onodera-punpun, I've fixed it :rocket: (as far I could check, at least)

After I relearn how to calculate the prayer times, I realized the algorithm that used on this package only worked on area with low latitude (below 48.5 degree from equator). Thanks to this, it failed in area with higher latitude where the sun does not go 18 degree below horizon on the summer.

Now this package allow users to specify method for calculating prayer times in higher latitude by setting field HighLatitudeMethod in Calculator. There are three methods for calculating times in area with high latitude but the most common one to use is angle-based which is the default in this package.

Unfortunately, I've had to change the API to simplify the code so you might need to upgrade your code. For example, now iqama time doesn't handled by this package anymore since it's simply adding duration to the prayer time.

With that said, here is the playground for the fixed version.

CamilleScholtz commented 4 years ago

Thanks a lot!