observerly / astrometry

observerly's lightweight, zero-dependency, type safe astrometry library written in TypeScript. This library can be used to ascertain to positions of the Sun, Moon and the planets, as well as ascertain transit times for any astronomical object.
https://jsr.io/@observerly/astrometry
MIT License
5 stars 0 forks source link

[BUG] v0.36.0 - getBodyNextSet method throw error #182

Closed EloxFire closed 3 months ago

EloxFire commented 4 months ago

Hello Michael,

While using @observerly/astrometry in my astronomy React Native application, I've come across an error when trying to get Rise and Set time of a target. The getBodyNextRise method seems to work perfectly, returning either a transitInstance or a boolean, but the getBodyNextSet on the other hand throws at me this error : Maximum call stack size exceeded which in React means that the app encountered an infinite loop somewhere.

Could you take a look ? I'm available if you need more informations !

According to my debugger, the error is located in astroshare-app/node_modules/@observerly/astrometry/dist/temporal.cjs

Here's the code snippet which work with the getBodyNextRise method :

import { EquatorialCoordinate, GeographicCoordinate, TransitInstance, getBodyNextRise, getBodyNextSet, isBodyAboveHorizon, isTransitInstance } from '@observerly/astrometry'
import { convertDMSToDegreeFromString } from '../helpers/scripts/astro/DmsToDegree'
import { convertHMSToDegreeFromString } from '../helpers/scripts/astro/HmsToDegree'
import { calculateHorizonAngle } from '../helpers/scripts/astro/calculateHorizonAngle'

const altitude = selectedSpot ? selectedSpot.equipments.altitude : defaultAltitude; // 342m est l'altitude moyenne en France métropolitaine
    const degRa = convertHMSToDegreeFromString(object.ra)
    const degDec = convertDMSToDegreeFromString(object.dec)
    const horizonAngle = calculateHorizonAngle(extractNumbers(altitude))

    if (degRa && degDec) {
      const observer: GeographicCoordinate = { latitude: currentUserLocation.lat, longitude: currentUserLocation.lon }
      const target: EquatorialCoordinate = { ra: degRa, dec: degDec }

      let visible = isBodyAboveHorizon(new Date(), observer, target, horizonAngle)
      let rise = getBodyNextRise(new Date(), observer, target, horizonAngle)

      if (isTransitInstance(rise)) {
        console.log(rise); // Outputs : {"GST": 11.947247845629565, "LST": 12.310635372296233, "az": 128.11534832444838, "datetime": 2024-05-20T18:01:18.216Z}
      }
    }

And here the code snippet that is not working with getBodyNextSet :

import { EquatorialCoordinate, GeographicCoordinate, TransitInstance, getBodyNextRise, getBodyNextSet, isBodyAboveHorizon, isTransitInstance } from '@observerly/astrometry'
import { convertDMSToDegreeFromString } from '../helpers/scripts/astro/DmsToDegree'
import { convertHMSToDegreeFromString } from '../helpers/scripts/astro/HmsToDegree'
import { calculateHorizonAngle } from '../helpers/scripts/astro/calculateHorizonAngle'

const altitude = selectedSpot ? selectedSpot.equipments.altitude : defaultAltitude; // 342m est l'altitude moyenne en France métropolitaine
    const degRa = convertHMSToDegreeFromString(object.ra)
    const degDec = convertDMSToDegreeFromString(object.dec)
    const horizonAngle = calculateHorizonAngle(extractNumbers(altitude))

    if (degRa && degDec) {
      const observer: GeographicCoordinate = { latitude: currentUserLocation.lat, longitude: currentUserLocation.lon }
      const target: EquatorialCoordinate = { ra: degRa, dec: degDec }

      let visible = isBodyAboveHorizon(new Date(), observer, target, horizonAngle)
      let set = getBodyNextSet(new Date(), observer, target, horizonAngle)

      if (isTransitInstance(set)) {
        console.log(set); // Outputs : Error 'Maximum call stack size exceeded'
      }
    }
EloxFire commented 4 months ago

UPDATE : It seems that the error can happen with getBodyNextRise too. But its quite random. For instance :

michealroberts commented 3 months ago

@EloxFire Hey Enzo, I will take a look at this bug this week! Apologies it has taken so long.

EloxFire commented 3 months ago

No worries ! Thanks a lot, take your time !

michealroberts commented 3 months ago

@EloxFire Can I also get the observer: GeographicCoordinate you use to calculate these errors, I can add some tests that way and ensure these pass as well as the existing ones ...

EloxFire commented 3 months ago

Of course !

const observer: GeographicCoordinate  = {
   latitude: 43.5314582,
   longitude: 5.4483161
}

And if you need this is my calculation of the horizon angle :

export const calculateHorizonAngle = (altitudeMeters: number): number => {
  const R = 6371; // Rayon de la Terre en kilomètres
  const h = altitudeMeters / 1000; // Convertir l'altitude de mètres en kilomètres

  // Calculer la dépression de l'horizon en radians
  const thetaRadians = Math.sqrt(2 * h / R);

  // Convertir la dépression en degrés
  const thetaDegrees = thetaRadians * (180 / Math.PI);

  return thetaDegrees;
};
const horizonAngle = calculateHorizonAngle(651) // Aprox 0.8190762287002356
michealroberts commented 3 months ago

Hey @EloxFire, so investigating this issue with the following test:

describe('getBodyNextRise and getBodyNextSet maximum call stack size error', () => {
  it('should not throw a maximum call stack size exceeded error for Messier 4', () => {
    const datetime = new Date('2024-05-24T02:03:04.568+00:00')

    const observer: GeographicCoordinate = {
      latitude: 43.5314582,
      longitude: 5.4483161
    }

    const target: EquatorialCoordinate = {
      ra: 246.275,
      dec: -26.58349
    }

    const set = getBodyNextSet(datetime, observer, target, 0.8190762287002356)

    console.log(set)

    expect(set).toBeDefined()

    const rise = getBodyNextRise(datetime, observer, target, 0.8190762287002356)

    console.log(rise)

    expect(rise).toBeDefined()
  })

  it('should not throw a maximum call stack size exceeded error for Messier 57', () => {
    const datetime = new Date('2024-05-24T02:03:04.568+00:00')

    const observer: GeographicCoordinate = {
      latitude: 43.5314582,
      longitude: 5.4483161
    }

    const target: EquatorialCoordinate = {
      ra: 283.395875,
      dec: 33.028583
    }

    const set = getBodyNextSet(datetime, observer, target, 0.8190762287002356)

    console.log(set)

    expect(set).toBeDefined()

    const rise = getBodyNextRise(datetime, observer, target, 0.8190762287002356)

    console.log(rise)

    expect(rise).toBeDefined()
  })
})

Yields the following:

stdout | tests/transit.spec.ts > getBodyNextSet > should not throw a Maximum call stack size exceeded error for Messier 4
{
  datetime: 2024-05-24T04:00:46.075Z,
  LST: 20.526032035806764,
  GST: 20.162810962473433,
  az: 231.884665713897
}
{
  datetime: 2024-05-24T19:45:15.514Z,
  LST: 12.310634630859909,
  GST: 11.947413557526575,
  az: 128.115334286103
}

stdout | tests/transit.spec.ts > getBodyNextSet > should not throw a Maximum call stack size exceeded error for Messier 57
{
  datetime: 2024-05-24T10:54:13.614Z,
  LST: 3.4358789771547933,
  GST: 3.07265790382146,
  az: 318.7471543851601
}
{
  datetime: 2024-05-24T17:47:57.351Z,
  LST: 10.350237689511872,
  GST: 9.987016616178538,
  az: 41.252845614839885
}

And the tests are passing ...

I have cross-referenced the Messier 4 case with the SkyGuide application (corrected for your approximate location of Marseille and +2 CET) is also correct:

messier4

I'm confident that the issue isn't ultimately down to @observerly/astrometry ... but I can't rule this out or explain why you're seeing this issue yet ...

Perhaps, it's an underlying issue with your wider setup ... are you still seeing this issue for the upgraded version of @observerly/astrometry v0.36.0?

EloxFire commented 3 months ago

I see, in that case I'll try to make some changes on my side and see if I can get deeper in this error. I will most likely have no time today, but I'll investigate more tomorrow.

Can you confirm that my data format is correct and everything I used in my code is used/placed in the right order ?

I will keep you posted if you don't mind ! Thanks a lot for taking the time to look into this, your job is just amazing.

EloxFire commented 3 months ago

And yes, I'm using v0.36.0 !

michealroberts commented 3 months ago

Hey Elox, perfect.

Yeh, looking through your code everything seems good ... I did see a similar error when using this package inside of a Vue component, but managed to fix it with some air gaps on the for loop.

I may see if I can look at optimisations for the code, but definitely let me know if you manage to get close to the truth ... it might be that we're looking in the wrong place, but anything you can do to replicate this error for me would be amazing. 😄

I'll await any further debugging you can look at.

michealroberts commented 3 months ago

P.S. Tracking tests for this issue on #188

EloxFire commented 3 months ago

Hi @michealroberts,

Sorry for the delay, big end of week at work so I did not have a minute for myself...

I digged deeper on my implementation of the getBodyNextRise and getBodyNextSet functions in my app. I can confirm (at 99.95% 😅 ) that there's nothing in the React-end that is causing the issue. I quadrupled-checked everything.

I took the liberty of copying your code from the repo, and using it in a customFile in my app in order to be able to add console logs and see what happens in it.

I made some changes to force the function to go all the way in its steps and force a valid return value :

The file :

import { EquatorialCoordinate, GeographicCoordinate, TransitInstance, convertGreenwhichSiderealTimeToUniversalTime, convertLocalSiderealTimeToGreenwhichSiderealTime, getBodyTransit, isBodyCircumpolar, isBodyVisible } from "@observerly/astrometry"

/**
 *
 * getBodyNextRise()
 *
 * Determines the next rise time for an object, if at all.
 *
 * @param date - The date to start searching for the next rise.
 * @param observer - The geographic coordinate of the observer.
 * @param target - The equatorial coordinate of the observed object.
 * @param horizon - The observer's horizon (in degrees).
 * @returns The next rise time or False if the object never rises, or True if the object is always above the horizon (circumpolar) for the observer.
 */
export const getBodyNextRise = (
  datetime: Date,
  observer: GeographicCoordinate,
  target: EquatorialCoordinate,
  horizon: number = 0
): TransitInstance | false => {
  const maxAttempts = 365; // Limit the number of attempts to prevent infinite recursion
  let attempts = 0;

  while (attempts < maxAttempts) {
    console.log(`Attempt ${attempts}: Checking rise time for: ${datetime}`);

    const tomorrow = new Date(
      datetime.getFullYear(),
      datetime.getMonth(),
      datetime.getDate() + 1,
      0,
      0,
      0,
      0
    );

    if (isBodyCircumpolar(observer, target, horizon) || !isBodyVisible(observer, target, horizon)) {
      console.log(`Object is circumpolar or not visible from the observer's location.`);
      return false;
    }

    const transit = getBodyTransit(observer, target);

    if (!transit) {
      console.log(`No transit found, checking the next day: ${tomorrow}`);
      datetime = tomorrow;
      attempts++;
      continue;
    }

    const LSTr = transit.LSTr;
    const GSTr = convertLocalSiderealTimeToGreenwhichSiderealTime(LSTr, observer);
    const rise = convertGreenwhichSiderealTimeToUniversalTime(GSTr, datetime);

    if (rise < datetime) {      
      console.log(`Rise time ${rise} is before current time, checking the next day: ${tomorrow}`);
      datetime = tomorrow;
      attempts++;
      continue;
    }

    console.log(`Next rise time found: ${rise}`);
    return {
      datetime: rise,
      LST: transit.LSTr,
      GST: GSTr,
      az: transit.R
    };
  }

  console.log(`Exceeded maximum attempts (${maxAttempts}) to find the next rise time.`);
  return false;
};

My implementation :

import { getBodyNextRise } from "../helpers/scripts/astro/getBodyRise";

export default function ObjectDetails({ route, navigation }: any) {

  const {currentUserLocation} = useSettings()
  const { object, isVisible } = route.params;
  const { selectedSpot, defaultAltitude } = useSpot()

  const [riseTime, setRiseTime] = useState<string | boolean>(false)
  const [setTime, setSetTime] = useState<string | boolean>(false)
  const [willRise, setWillRise] = useState<boolean>(false)

  useEffect(() => {
    const altitude = selectedSpot ? selectedSpot.equipments.altitude : defaultAltitude;
    const degRa = convertHMSToDegreeFromString(object.ra)
    const degDec = convertDMSToDegreeFromString(object.dec)
    const horizonAngle = calculateHorizonAngle(extractNumbers(altitude))

    if (!degRa || !degDec || !horizonAngle) return;
    const target: EquatorialCoordinate = { ra: degRa, dec: degDec }
    const observer: GeographicCoordinate = { latitude: currentUserLocation.lat, longitude: currentUserLocation.lon }

    setWillRise(isBodyVisibleForNight(new Date(), observer, target, horizonAngle))
    console.log(getBodyNextRise(new Date(), observer, target, horizonAngle));     
  }, [])

  return (
    <View style={globalStyles.body}>
       [... some more code]
    </View>
  );
}

And here's what I found so far : The function is always returning a rise time before the current time, so it's trying to call itself for the next day, but re-returns a rise (or set) time before the new date..

Here are some logs for you to see what I'm talking about :

Attempt 0: Checking rise time for: Sat Jun 01 2024 00:23:58 GMT+0200
 LOG  Rise time Fri May 31 2024 05:05:55 GMT+0200 is before current time, checking the next day: Sun Jun 02 2024 00:00:00 GMT+0200
 LOG  Attempt 1: Checking rise time for: Sun Jun 02 2024 00:00:00 GMT+0200
 LOG  Rise time Sat Jun 01 2024 05:01:59 GMT+0200 is before current time, checking the next day: Mon Jun 03 2024 00:00:00 GMT+0200
 LOG  Attempt 2: Checking rise time for: Mon Jun 03 2024 00:00:00 GMT+0200
 LOG  Rise time Sun Jun 02 2024 04:58:03 GMT+0200 is before current time, checking the next day: Tue Jun 04 2024 00:00:00 GMT+0200
 LOG  Attempt 3: Checking rise time for: Tue Jun 04 2024 00:00:00 GMT+0200
 LOG  Rise time Mon Jun 03 2024 04:54:07 GMT+0200 is before current time, checking the next day: Wed Jun 05 2024 00:00:00 GMT+0200
 LOG  Attempt 4: Checking rise time for: Wed Jun 05 2024 00:00:00 GMT+0200
 LOG  Rise time Tue Jun 04 2024 04:50:12 GMT+0200 is before current time, checking the next day: Thu Jun 06 2024 00:00:00 GMT+0200
 LOG  Attempt 5: Checking rise time for: Thu Jun 06 2024 00:00:00 GMT+0200
 LOG  Rise time Wed Jun 05 2024 04:46:16 GMT+0200 is before current time, checking the next day: Fri Jun 07 2024 00:00:00 GMT+0200
 LOG  Attempt 6: Checking rise time for: Fri Jun 07 2024 00:00:00 GMT+0200
 LOG  Rise time Thu Jun 06 2024 04:42:20 GMT+0200 is before current time, checking the next day: Sat Jun 08 2024 00:00:00 GMT+0200
 LOG  Attempt 7: Checking rise time for: Sat Jun 08 2024 00:00:00 GMT+0200
 LOG  Rise time Fri Jun 07 2024 04:38:24 GMT+0200 is before current time, checking the next day: Sun Jun 09 2024 00:00:00 GMT+0200
 LOG  Attempt 8: Checking rise time for: Sun Jun 09 2024 00:00:00 GMT+0200
 LOG  Rise time Sat Jun 08 2024 04:34:28 GMT+0200 is before current time, checking the next day: Mon Jun 10 2024 00:00:00 GMT+0200
 LOG  Attempt 9: Checking rise time for: Mon Jun 10 2024 00:00:00 GMT+0200
 LOG  Rise time Sun Jun 09 2024 04:30:32 GMT+0200 is before current time, checking the next day: Tue Jun 11 2024 00:00:00 GMT+0200
 LOG  Attempt 10: Checking rise time for: Tue Jun 11 2024 00:00:00 GMT+0200
 LOG  Rise time Mon Jun 10 2024 04:26:36 GMT+0200 is before current time, checking the next day: Wed Jun 12 2024 00:00:00 GMT+0200
 LOG  Attempt 11: Checking rise time for: Wed Jun 12 2024 00:00:00 GMT+0200
 LOG  Rise time Tue Jun 11 2024 04:22:40 GMT+0200 is before current time, checking the next day: Thu Jun 13 2024 00:00:00 GMT+0200
 LOG  Attempt 12: Checking rise time for: Thu Jun 13 2024 00:00:00 GMT+0200
 LOG  Rise time Wed Jun 12 2024 04:18:44 GMT+0200 is before current time, checking the next day: Fri Jun 14 2024 00:00:00 GMT+0200
 LOG  Attempt 13: Checking rise time for: Fri Jun 14 2024 00:00:00 GMT+0200
 LOG  Rise time Thu Jun 13 2024 04:14:48 GMT+0200 is before current time, checking the next day: Sat Jun 15 2024 00:00:00 GMT+0200
 LOG  Attempt 14: Checking rise time for: Sat Jun 15 2024 00:00:00 GMT+0200
 LOG  Rise time Fri Jun 14 2024 04:10:52 GMT+0200 is before current time, checking the next day: Sun Jun 16 2024 00:00:00 GMT+0200
 LOG  Attempt 15: Checking rise time for: Sun Jun 16 2024 00:00:00 GMT+0200
 LOG  Rise time Sat Jun 15 2024 04:06:57 GMT+0200 is before current time, checking the next day: Mon Jun 17 2024 00:00:00 GMT+0200
 LOG  Attempt 16: Checking rise time for: Mon Jun 17 2024 00:00:00 GMT+0200
 LOG  Rise time Sun Jun 16 2024 04:03:01 GMT+0200 is before current time, checking the next day: Tue Jun 18 2024 00:00:00 GMT+0200
 LOG  Attempt 17: Checking rise time for: Tue Jun 18 2024 00:00:00 GMT+0200
 LOG  Rise time Mon Jun 17 2024 03:59:05 GMT+0200 is before current time, checking the next day: Wed Jun 19 2024 00:00:00 GMT+0200
 LOG  Attempt 18: Checking rise time for: Wed Jun 19 2024 00:00:00 GMT+0200

And it goes like this until attempt 365 - the maximum call stack size error :

 LOG  Rise time Fri May 30 2025 05:10:48 GMT+0200 is before current time, checking the next day: Sun Jun 01 2025 00:00:00 GMT+0200
 LOG  Exceeded maximum attempts (365) to find the next rise time.
 LOG  false`

I am not comfortable with astronomical calculations and conversions to Julian days but do you think that some mistake can happen in convertLocalSiderealTimeToGreenwhichSiderealTime or convertGreenwhichSiderealTimeToUniversalTime ?

Here's a sample of my functions props :

{
   "datetime": 2024-05-31T22:38:21.424Z,
   "horizon": 0.8190762287002356,
   "observer": {
      "latitude": 43.531456,
      "longitude": 5.4507896
   },
   "target": {
      "dec": 22.01447222222222,
      "ra": 83.63320833333333
   }
}

I'm sorry that I can't provide more infos or debug leads, it's the first time I really am able to have an impact on an under-development project so I'm not used to make this kind of issue traking.

I hope this post his helping and I'm available if you need more data ! Also I hope this issue is not holding you back on some other stuff, otherwise it can wait for sure ;)

Thanks a lot, as always ! Enzo

michealroberts commented 3 months ago

Hey @EloxFire I have double checked the code my side, and have added the sample parameters you have provided to some extra tests, i.e.:

{
   "datetime": 2024-05-31T22:38:21.424Z,
   "horizon": 0.8190762287002356,
   "observer": {
      "latitude": 43.531456,
      "longitude": 5.4507896
   },
   "target": {
      "dec": 22.01447222222222,
      "ra": 83.63320833333333
   }
}

I don't see an error with the new tests added here: #193 , and I have also looked at the code you have provided and it feels like you are looking at old code from an old version of this library.

I am specifically looking at one check in your code you have added:

if (rise < datetime) {
   ... // increments the datetime etc
}

This was amended to be more robust in this bug fix commit here: https://github.com/observerly/astrometry/commit/da8c6f69da532aa4cc78e0624638a34def26b18b

So, given the most recent release is v0.38.0, I would recommend seeing if the code you have in your local node_modules is out of date ... (sometimes the package.json has the correct version, but your node_modules need to be refreshed).

With pnpm, you can amend the version number in situ in the package.json and then re-run ppm install in your workspace root.

Let me know if that fixes the issues you're seeing.

EloxFire commented 3 months ago

Hi !

Sorry for the confusion, the little part your amended in commit https://github.com/observerly/astrometry/commit/da8c6f69da532aa4cc78e0624638a34def26b18b was here when I added my logs, the missing .getTime() is my fault, I removed them for a small test and forgot to put them back when writing the comment. The error was still there even with them in the condition.

I have upgraded to v0.38.0 and checked if the code present in node_modules is correct and I did not see any mismatch, nor in version number in the package.json and in the code (I search for the new convertHorizontalToEquatorial to make sure I was with the right version)

The error is still here unfortunatly...

michealroberts commented 3 months ago

Hey @EloxFire Enzo, the code you have supplied here seems to be out of date with the base repository, so I'm not 100% sure you have the most recent changes.

The while loop over a full year has now gone entirely, and is more performant to just cycle through provided an object is always going to be able to be above the horizon.

EloxFire commented 3 months ago

Hum the while loop is also something I added, to keep track of the attempts and stop the function to be able to see the logs before the error.

This is observerly's state in my node_modules : image

{
  "name": "@observerly/astrometry",
  "version": "0.38.0",
  "description": "observerly's lightweight, zero-dependency, type safe astrometry library written in Typescript for calculating the position of celestial objects in the sky.",
  "private": false,
  "license": "MIT",
  "repository": {
    "type": "git",
    "url": "git+https://github.com/observerly/astrometry.git"
  },
  "author": "Micheal J. Roberts",
  "prepublish": "tsc",
  "publishConfig": {
    "registry": "https://npm.pkg.github.com/observerly"
  },
  "type": "module",
  "files": [
    "dist"
  ],
  "keywords": [
    "astronomy",
    "astrometry",
    "stars",
    "galaxies",
    "planets",
    "moon",
    "sun",
    "ephemeris",
    "almanac",
    "transit",
    "eclipse",
    "conjunction",
    "observer",
    "observerly"
  ],
  "main": "dist/index.cjs",
  "module": "dist/index.js",
  "types": "dist/index.d.ts",
  "exports": {
    ".": {
      "import": "./dist/index.js",
      "require": "./dist/index.cjs",
      "types": "./dist/index.d.ts"
    },
    "./abberation": {
      "import": "./dist/abberation.js",
      "require": "./dist/abberation.cjs",
      "types": "./dist/abberation.d.ts"
    },
    "./astrometry": {
      "import": "./dist/astrometry.js",
      "require": "./dist/astrometry.cjs",
      "types": "./dist/astrometry.d.ts"
    },
    "./common": {
      "import": "./dist/common.js",
      "require": "./dist/common.cjs",
      "types": "./dist/common.d.ts"
    },
    "./constants": {
      "import": "./dist/constants.js",
      "require": "./dist/constants.cjs",
      "types": "./dist/constants.d.ts"
    },
    "./constellations": {
      "import": "./dist/constellations.js",
      "require": "./dist/constellations.cjs",
      "types": "./dist/constellations.d.ts"
    },
    "./coordinates": {
      "import": "./dist/coordinates.js",
      "require": "./dist/coordinates.cjs",
      "types": "./dist/coordinates.d.ts"
    },
    "./earth": {
      "import": "./dist/earth.js",
      "require": "./dist/earth.cjs",
      "types": "./dist/earth.d.ts"
    },
    "./eclipse": {
      "import": "./dist/eclipse.js",
      "require": "./dist/eclipse.cjs",
      "types": "./dist/eclipse.d.ts"
    },
    "./ecliptic": {
      "import": "./dist/ecliptic.js",
      "require": "./dist/ecliptic.cjs",
      "types": "./dist/ecliptic.d.ts"
    },
    "./epoch": {
      "import": "./dist/epoch.js",
      "require": "./dist/epoch.cjs",
      "types": "./dist/epoch.d.ts"
    },
    "./galactic": {
      "import": "./dist/galactic.js",
      "require": "./dist/galactic.cjs",
      "types": "./dist/galactic.d.ts"
    },
    "./humanize": {
      "import": "./dist/humanize.js",
      "require": "./dist/humanize.cjs",
      "types": "./dist/humanize.d.ts"
    },
    "./iers": {
      "import": "./dist/iers.js",
      "require": "./dist/iers.cjs",
      "types": "./dist/iers.d.ts"
    },
    "./moon": {
      "import": "./dist/moon.js",
      "require": "./dist/moon.cjs",
      "types": "./dist/moon.d.ts"
    },
    "./night": {
      "import": "./dist/night.js",
      "require": "./dist/night.cjs",
      "types": "./dist/night.d.ts"
    },
    "./nutation": {
      "import": "./dist/nutation.js",
      "require": "./dist/nutation.cjs",
      "types": "./dist/nutation.d.ts"
    },
    "./observation": {
      "import": "./dist/observation.js",
      "require": "./dist/observation.cjs",
      "types": "./dist/observation.d.ts"
    },
    "./observer": {
      "import": "./dist/observer.js",
      "require": "./dist/observer.cjs",
      "types": "./dist/observer.d.ts"
    },
    "./orbit": {
      "import": "./dist/orbit.js",
      "require": "./dist/orbit.cjs",
      "types": "./dist/orbit.d.ts"
    },
    "./planets": {
      "import": "./dist/planets.js",
      "require": "./dist/planets.cjs",
      "types": "./dist/planets.d.ts"
    },
    "./precession": {
      "import": "./dist/precession.js",
      "require": "./dist/precession.cjs",
      "types": "./dist/precession.d.ts"
    },
    "./projection": {
      "import": "./dist/projection.js",
      "require": "./dist/projection.cjs",
      "types": "./dist/projection.d.ts"
    },
    "./refraction": {
      "import": "./dist/refraction.js",
      "require": "./dist/refraction.cjs",
      "types": "./dist/refraction.d.ts"
    },
    "./seeing": {
      "import": "./dist/seeing.js",
      "require": "./dist/seeing.cjs",
      "types": "./dist/seeing.d.ts"
    },
    "./sun": {
      "import": "./dist/sun.js",
      "require": "./dist/sun.cjs",
      "types": "./dist/sun.d.ts"
    },
    "./temporal": {
      "import": "./dist/temporal.js",
      "require": "./dist/temporal.cjs",
      "types": "./dist/temporal.d.ts"
    },
    "./transit": {
      "import": "./dist/transit.js",
      "require": "./dist/transit.cjs",
      "types": "./dist/transit.d.ts"
    }
  },
  "release": {
    "branches": [
      "main"
    ]
  },
  "scripts": {
    "build": "tsc --noEmit && vite build",
    "format": "prettier \"src/**/*.ts\" --write",
    "lint": "eslint src --ext .ts --fix",
    "test": "vitest",
    "coverage": "vitest run --coverage"
  },
  "devDependencies": {
    "@changesets/cli": "^2.26.2",
    "@rollup/plugin-typescript": "^11.1.6",
    "@types/node": "^20.11.19",
    "@typescript-eslint/eslint-plugin": "^6.3.0",
    "@vitest/coverage-v8": "^1.3.0",
    "eslint": "^8.47.0",
    "eslint-config-prettier": "^9.0.0",
    "eslint-plugin-prettier": "^5.0.0",
    "husky": "^8.0.3",
    "lint-staged": "^13.2.3",
    "path": "^0.12.7",
    "prettier": "3.0.1",
    "rollup": "^4.14.1",
    "tslib": "^2.6.1",
    "typescript": "5.3.3",
    "vite": "^5.2.8",
    "vitest": "^1.3.0"
  },
  "husky": {
    "hooks": {
      "pre-commit": "lint-staged"
    }
  },
  "lint-staged": {
    "*.{ts,tsx}": [
      "prettier --write",
      "eslint --fix",
      "git add -A ."
    ]
  }
}

I did a run with the un-altered version directly copied/pasted from the transit.js file on Github, I just added some logs here and there :

import {
  EquatorialCoordinate,
  GeographicCoordinate,
  TransitInstance,
  convertGreenwhichSiderealTimeToUniversalTime,
  convertLocalSiderealTimeToGreenwhichSiderealTime,
  getBodyTransit,
  isBodyCircumpolar,
  isBodyVisible
} from "@observerly/astrometry"

/**
 *
 * getBodyNextRise()
 *
 * Determines the next rise time for an object, if at all.
 *
 * @param date - The date to start searching for the next rise.
 * @param observer - The geographic coordinate of the observer.
 * @param target - The equatorial coordinate of the observed object.
 * @param horizon - The observer's horizon (in degrees).
 * @returns The next rise time or False if the object never rises, or True if the object is always above the horizon (circumpolar) for the observer.
 */
export const getBodyNextRise = (
  datetime: Date,
  observer: GeographicCoordinate,
  target: EquatorialCoordinate,
  horizon: number = 0
): TransitInstance | false => {
  console.log("In getBodyNextRise :", datetime, observer, target, horizon); // LOG ADDED

  const tomorrow = new Date(
    datetime.getFullYear(),
    datetime.getMonth(),
    datetime.getDate() + 1,
    0,
    0,
    0,
    0
  )

  console.log("Tomorrow :", tomorrow);

  // If the object is circumpolar, or it is not visible from the observer's location, it never rises:
  if (isBodyCircumpolar(observer, target, horizon) || !isBodyVisible(observer, target, horizon)) {
    console.log("Body is circumpolar or not visible.") // LOG ADDED
    return false
  }

  const transit = getBodyTransit(observer, target)

  if (!transit) {
    // Get the next rise time for the next day:
    console.log("No transit found."); // LOG ADDED
    return getBodyNextRise(tomorrow, observer, target, horizon)
  }

  const LSTr = transit.LSTr

  // Convert the local sidereal time of rise to Greenwhich sidereal time:
  const GSTr = convertLocalSiderealTimeToGreenwhichSiderealTime(LSTr, observer)

  // Convert the Greenwhich sidereal time to universal coordinate time for the date specified:
  const rise = convertGreenwhichSiderealTimeToUniversalTime(GSTr, datetime)

  console.log("Rise time :", rise); // LOG ADDED

  // If the rise is before the current time, then we know the next rise is tomorrow:
  if (rise.getTime() < datetime.getTime()) {
    console.log("Rise is before current time."); // LOG ADDED
    return getBodyNextRise(tomorrow, observer, target, horizon)
  }

  return {
    datetime: rise,
    LST: transit.LSTr,
    GST: GSTr,
    az: transit.R
  }
}

And this is an output sample :

 LOG  In getBodyNextRise : 2024-06-01T14:25:15.846Z {"latitude": 43.5314558, "longitude": 5.4507789} {"dec": 22.01447222222222, "ra": 83.63320833333333} 0.8190762287002356
 LOG  Tomorrow : 2024-06-01T22:00:00.000Z
 LOG  Rise time : 2024-06-01T03:01:59.750Z
 LOG  Rise is before current time.
 LOG  In getBodyNextRise : 2024-06-01T22:00:00.000Z {"latitude": 43.5314558, "longitude": 5.4507789} {"dec": 22.01447222222222, "ra": 83.63320833333333} 0.8190762287002356
 LOG  Tomorrow : 2024-06-02T22:00:00.000Z
 LOG  Rise time : 2024-06-01T03:01:59.750Z
 LOG  Rise is before current time.
 LOG  In getBodyNextRise : 2024-06-02T22:00:00.000Z {"latitude": 43.5314558, "longitude": 5.4507789} {"dec": 22.01447222222222, "ra": 83.63320833333333} 0.8190762287002356
 LOG  Tomorrow : 2024-06-03T22:00:00.000Z
 LOG  Rise time : 2024-06-02T02:58:03.841Z
 LOG  Rise is before current time.

It goes like this until :

 LOG  Rise time : 2030-01-23T12:30:49.142Z
 LOG  Rise is before current time.
 LOG  In getBodyNextRise : 2030-01-24T23:00:00.000Z {"latitude": 43.5314558, "longitude": 5.4507789} {"dec": 22.01447222222222, "ra": 83.63320833333333} 0.8190762287002356
 ERROR  RangeError: Maximum call stack size exceeded
michealroberts commented 3 months ago

@EloxFire Ok, interesting. I think it's a timezone issue then.

Could you for me change the tomorrow declaration to use their UTC counterparts, e.g., datetime.getUTCFullYear() etc

EloxFire commented 3 months ago

Here it is

Changed the tomorrow declaration like so :

const tomorrow = new Date(
    datetime.getUTCFullYear(),
    datetime.getUTCMonth(),
    datetime.getUTCDate() + 1,
    0,
    0,
    0,
    0
  )

Here's the log output :

In getBodyNextRise : 2024-06-01T14:44:40.684Z {"latitude": 43.5314435, "longitude": 5.4507871} {"dec": 22.01447222222222, "ra": 83.63320833333333} 0.8190762287002356
 LOG  Tomorrow : 2024-06-01T22:00:00.000Z
 LOG  Rise time : 2024-06-01T03:01:59.751Z
 LOG  Rise is before current time.
 LOG  In getBodyNextRise : 2024-06-01T22:00:00.000Z {"latitude": 43.5314435, "longitude": 5.4507871} {"dec": 22.01447222222222, "ra": 83.63320833333333} 0.8190762287002356
 LOG  Tomorrow : 2024-06-01T22:00:00.000Z
 LOG  Rise time : 2024-06-01T03:01:59.751Z
 LOG  Rise is before current time.
 LOG  In getBodyNextRise : 2024-06-01T22:00:00.000Z {"latitude": 43.5314435, "longitude": 5.4507871} {"dec": 22.01447222222222, "ra": 83.63320833333333} 0.8190762287002356
 LOG  Tomorrow : 2024-06-01T22:00:00.000Z
 LOG  Rise time : 2024-06-01T03:01:59.751Z
 LOG  Rise is before current time.

Still hiting the call stack error...

Do I need to pass a UTC formated date directly in getBodyNextRise(), ? Instead of just anew Date()` ?

michealroberts commented 3 months ago

@EloxFire You shouldn't have to ... ideally.

I must admit, I am stumped, I have ran various different examples through tests, and have amended the timezone to CET (Europe/Paris) via process.env.TZ and I can not replicate the behaviour you are seeing here :(

That's not to dismiss this, I just can not replicate it. I will persevere to try and understand the issue, but we have been using this library in production with observably for approximately a year and I have yet to see this error.

michealroberts commented 3 months ago

@EloxFire I'm wondering, is it possible to see the full codebase?

I think my tests are missing some facet, it would be interesting to see if it also works in a different timezone.

EloxFire commented 3 months ago

Of course !

https://github.com/EloxFire/astroshare-app

getBodyNextRise is used in src/screens/ObjectDetails.tsx

If you have an android phone you'll have to download Expo Go app on the playstore. Otherwise you only have to provide a .env file at the root with at least : EXPO_PUBLIC_ASTROSHARE_API_URL=https://api.astroshare.fr for the API :)

EloxFire commented 3 months ago

I understand your frustration. And I am myself quite astonished by this behavior. Maybe your eyes on my codebase will reveal something I'm missing since I know this code a bit to much ^^, some external view may be good !

I want to thank you again for your time and efforts 🙏🏻 And if we do not find something interesting from my codebase, I suggest we let this cool a moment and maybe I'll comeback on it with a new insight :)

EloxFire commented 3 months ago

Okay, I have some news !

I found a workaround to the timezone issue (for now) :

const tomorrow = new Date(
    datetime.getFullYear(),
    datetime.getMonth(),
    datetime.getDate() + 1,
    2,
    0,
    0,
    1
  )

Instead of :

const tomorrow = new Date(
    datetime.getFullYear(),
    datetime.getMonth(),
    datetime.getDate() + 1,
    0,
    0,
    0,
    0
  )

This works only in France and places where the timezone has +2h from Greenwich obviously... But it works for me since my app be will only available for french users (for now).

It clearly is not a very clean solution but it's enough for for and for now.

michealroberts commented 3 months ago

Hey @EloxFire Enzo, I have been looking over various date time specifications and putting in some research, could you confirm if amending tomorrow to this works:

const tomorrow = new Date(
    Date.UTC(
      datetime.getUTCFullYear(),
      datetime.getUTCMonth(),
      datetime.getUTCDate() + 1,
      0,
      0,
      0,
      0
    )
  )
EloxFire commented 3 months ago

Hi @michealroberts,

Yes it works !

I'm getting accurate rise and set times, the error is gone and the if statements are looping in a normal way !

michealroberts commented 3 months ago

@EloxFire Perfect, I will extend the test suite for different timezones other than UTC to stabilise browser support across locL user timezones.

I will push a fix today, and work towards v0.39.0 with improved stability.

EloxFire commented 3 months ago

Amazing !

Again thanks a lot for your time !

michealroberts commented 3 months ago

@EloxFire Releasing a fix for this in v0.38.1.

michealroberts commented 3 months ago

@EloxFire Could you let me know if you see any further issues with v0.38.1 on the rise and set times. 🙏