Skip to main content
added 1 character in body
Source Link
chux
  • 36.4k
  • 2
  • 43
  • 97

More documentation

Functions in src/core.h deserve details for their valid range of year, month, day and Julian day.

Somewhere an explanation or reference of julian_day is deserved.

In particular, what is the year before 1 (1 AD)? Is it 0 or -1 (1 BC)?

Range

Julian day deserves to be wider than int as int may only be 16-bit. Consider long or int32_t, etc.

Precession

Julian day is fundamentally 0.5 off from a calendar date. Julian day 0.0 is noon of Monday, January 1, 4713 BC, proleptic Julian calendar.

Converting dates (year, month, day) to/from Julian day encounter "off-by 0.5" due to this.

An alternative is to use Modified JD which reckons from midnight and not noon.

Correctness

AVG_DAYS_PER_YEAR = 365.25 looks amiss as theit is wrong for years since 1582. AVG_DAYS_PER_YEAR = 365.2425 expected. Yet code has cryptic margin = (double)((int)(year / 100)); which might account for this.

In any case, AVG_DAYS_PER_YEAR better as AVG_DAYS_PER_JULIAN_YEAR.

Credit

I suspect OP lifted the formula from someone's prior work - if so, a reference to that algorithm is a must.

Unnamed constants

Constants like 1720994.5 deserve to be named like:

// Some appropriate name.
#define Days_From_4716_To_AD1Jan1Days_From_4716BC_To_AD1Jan1 1720994.5 

Other naked constants include: 3, 2, 4, 0.75.

Name Space

Why is core defining array_from_date_range, julian_day? A more uniform naming is deserved.

e.g. astro_core.h, astro_jd_array_from_date_range, astro_julian_day.

Why cast to int?

(double)((int)(year / 100)) is unnecessarily restrictive to the int range. Equivalent code is trunc(year / 100) and certainly less complex.

Edge cases

I have doubts that code is functionally correct near edge dates like Jan 1, 1 AD and March 1, 1 AD. A typical source of error is lifting the algorithm that needs mod and using truncate which is what casting to int does.

Perhaps supply the source of the algorithm?

More documentation

Functions in src/core.h deserve details for their valid range of year, month, day and Julian day.

Somewhere an explanation or reference of julian_day is deserved.

In particular, what is the year before 1 (1 AD)? Is it 0 or -1 (1 BC)?

Range

Julian day deserves to be wider than int as int may only be 16-bit. Consider long or int32_t, etc.

Precession

Julian day is fundamentally 0.5 off from a calendar date. Julian day 0.0 is noon of Monday, January 1, 4713 BC, proleptic Julian calendar.

Converting dates (year, month, day) to/from Julian day encounter "off-by 0.5" due to this.

An alternative is to use Modified JD which reckons from midnight and not noon.

Correctness

AVG_DAYS_PER_YEAR = 365.25 looks amiss as the is wrong for years since 1582. AVG_DAYS_PER_YEAR = 365.2425 expected. Yet code has cryptic margin = (double)((int)(year / 100)); which might account for this.

In any case, AVG_DAYS_PER_YEAR better as AVG_DAYS_PER_JULIAN_YEAR.

Credit

I suspect OP lifted the formula from someone's prior work - if so, a reference to that algorithm is a must.

Unnamed constants

Constants like 1720994.5 deserve to be named like:

// Some appropriate name.
#define Days_From_4716_To_AD1Jan1 1720994.5 

Other naked constants include: 3, 2, 4, 0.75.

Name Space

Why is core defining array_from_date_range, julian_day? A more uniform naming is deserved.

e.g. astro_core.h, astro_jd_array_from_date_range, astro_julian_day.

Why cast to int?

(double)((int)(year / 100)) is unnecessarily restrictive to the int range. Equivalent code is trunc(year / 100) and certainly less complex.

Edge cases

I have doubts that code is functionally correct near edge dates like Jan 1, 1 AD and March 1, 1 AD. A typical source of error is lifting the algorithm that needs mod and using truncate which is what casting to int does.

Perhaps supply the source of the algorithm?

More documentation

Functions in src/core.h deserve details for their valid range of year, month, day and Julian day.

Somewhere an explanation or reference of julian_day is deserved.

In particular, what is the year before 1 (1 AD)? Is it 0 or -1 (1 BC)?

Range

Julian day deserves to be wider than int as int may only be 16-bit. Consider long or int32_t, etc.

Precession

Julian day is fundamentally 0.5 off from a calendar date. Julian day 0.0 is noon of Monday, January 1, 4713 BC, proleptic Julian calendar.

Converting dates (year, month, day) to/from Julian day encounter "off-by 0.5" due to this.

An alternative is to use Modified JD which reckons from midnight and not noon.

Correctness

AVG_DAYS_PER_YEAR = 365.25 looks amiss as it is wrong for years since 1582. AVG_DAYS_PER_YEAR = 365.2425 expected. Yet code has cryptic margin = (double)((int)(year / 100)); which might account for this.

In any case, AVG_DAYS_PER_YEAR better as AVG_DAYS_PER_JULIAN_YEAR.

Credit

I suspect OP lifted the formula from someone's prior work - if so, a reference to that algorithm is a must.

Unnamed constants

Constants like 1720994.5 deserve to be named like:

// Some appropriate name.
#define Days_From_4716BC_To_AD1Jan1 1720994.5 

Other naked constants include: 3, 2, 4, 0.75.

Name Space

Why is core defining array_from_date_range, julian_day? A more uniform naming is deserved.

e.g. astro_core.h, astro_jd_array_from_date_range, astro_julian_day.

Why cast to int?

(double)((int)(year / 100)) is unnecessarily restrictive to the int range. Equivalent code is trunc(year / 100) and certainly less complex.

Edge cases

I have doubts that code is functionally correct near edge dates like Jan 1, 1 AD and March 1, 1 AD. A typical source of error is lifting the algorithm that needs mod and using truncate which is what casting to int does.

Perhaps supply the source of the algorithm?

Source Link
chux
  • 36.4k
  • 2
  • 43
  • 97

More documentation

Functions in src/core.h deserve details for their valid range of year, month, day and Julian day.

Somewhere an explanation or reference of julian_day is deserved.

In particular, what is the year before 1 (1 AD)? Is it 0 or -1 (1 BC)?

Range

Julian day deserves to be wider than int as int may only be 16-bit. Consider long or int32_t, etc.

Precession

Julian day is fundamentally 0.5 off from a calendar date. Julian day 0.0 is noon of Monday, January 1, 4713 BC, proleptic Julian calendar.

Converting dates (year, month, day) to/from Julian day encounter "off-by 0.5" due to this.

An alternative is to use Modified JD which reckons from midnight and not noon.

Correctness

AVG_DAYS_PER_YEAR = 365.25 looks amiss as the is wrong for years since 1582. AVG_DAYS_PER_YEAR = 365.2425 expected. Yet code has cryptic margin = (double)((int)(year / 100)); which might account for this.

In any case, AVG_DAYS_PER_YEAR better as AVG_DAYS_PER_JULIAN_YEAR.

Credit

I suspect OP lifted the formula from someone's prior work - if so, a reference to that algorithm is a must.

Unnamed constants

Constants like 1720994.5 deserve to be named like:

// Some appropriate name.
#define Days_From_4716_To_AD1Jan1 1720994.5 

Other naked constants include: 3, 2, 4, 0.75.

Name Space

Why is core defining array_from_date_range, julian_day? A more uniform naming is deserved.

e.g. astro_core.h, astro_jd_array_from_date_range, astro_julian_day.

Why cast to int?

(double)((int)(year / 100)) is unnecessarily restrictive to the int range. Equivalent code is trunc(year / 100) and certainly less complex.

Edge cases

I have doubts that code is functionally correct near edge dates like Jan 1, 1 AD and March 1, 1 AD. A typical source of error is lifting the algorithm that needs mod and using truncate which is what casting to int does.

Perhaps supply the source of the algorithm?