-
Notifications
You must be signed in to change notification settings - Fork 244
Improve the docs of Hijri AstronomicalSimulation #7325
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
base: main
Are you sure you want to change the base?
Conversation
Manishearth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good to get things clearly written down at least
components/calendar/src/cal/hijri.rs
Outdated
| /// The simulations use the relative positions of the Earth, moon, and sun to predict the | ||
| /// exact moment a new moon occurs. Because this is rarely the instant when a crescent | ||
| /// sighting occurs, the month start dates preducted by these rules will often be one or | ||
| /// more days earlier than actually observed. Applications using these rules should have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe say something like "These Rules shouldn't be used directly, but can form the basis for a Rules implementation based on human sightings". The correction should be applied at the Rules level, not at the Date level (but I don't really see the point of using this if you can just write a sighting Rules).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made it say:
These rules can form the basis of a custom [
Rules] implementation that includes data based on human sightings. As discussed in the [Hijri] documentation, using these rules without allowing for adjustments will produce dates that are only approximations of the ground truth.
| /// such predictions only an approximation of ground truth. Hijri calendar applications should | ||
| /// therefore include the ability to specify a crescent sighting adjustment. | ||
| /// | ||
| /// The primary exception is Saudi Arabia, where the KACST uses sophisticated telescopes to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do sophisticated telescopes and observations have to do with UAQ? I also wouldn't necessarily call them "predictions", they are correct by definition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They still claim to do observations. The calculations predict what their fancy infrared telescopes will see. I don't know what happens if they can't observe their prediction; I think they just claim that it won't ever happen, and they go to extreme measures to make sure that the crescent is visible when they claim it will be.
This is a good article on the subject:
https://www.middleeasteye.net/news/eid-2025-does-saudi-arabia-report-impossible-moonsightings
Here's an example image of a crescent sighting done with this method:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That article literally says that their observation claim is BS because they've claimed impossible sightings.
Yet astronomers say it will be impossible to see the moon - even with optical aid, such as telescopes - on Saturday.
prominent Kuwaiti astronomer Adel al-Saadoun declared that it was “impossible to see the crescent this evening” in the Arabian Peninsula.
Last year, in 2024, Saudi Arabia announced on 6 June that Eid ul-Adha (the second Eid of the year) would commence in ten days after the new crescent moon for the month of Dhu al-Hijjah was sighted.
That was despite astronomical bodies insisting it was impossible for the moon to have been sighted.
The UAE-based International Astronomy Centre has likewise explained that in the Middle East it will be impossible to see the crescent on Saturday, even with new technology.
It also clearly states that UAQ is a calculated calendar.
Saudi Arabia uses a calendar called the Umm al-Qura, which is based on calculations and marks key dates years in advance.
“Turkey’s calendar is pre-calculated and their formula is more or less the same as Saudi Arabia’s,” Ahmed explained.
“But they’re transparent. They don’t claim they've seen the moon, like Saudi do. They're clear about what their formula is.”
I don't think we should echo SA's scientifically dubious claims and just state UAQ as what it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, fair point. I changed it to: "The primary exception is Saudi Arabia, where the KACST publishes official predictions of crescent timings for multiple centuries."
Moved to #7330
components/calendar/src/cal/hijri.rs
Outdated
| /// [`UmmAlQura`], which uses the results of KACST's Mecca-based calculations. | ||
| /// These rules use the relative positions of the Earth, moon, and sun to predict the | ||
| /// exact moment of a new moon, and then apply a criterion proposed by S. K. Shaukat in | ||
| /// 1996 to determine crescent moon visibility. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
link?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a citation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
linking to an undocumented method is not a citation
components/calendar/src/cal/hijri.rs
Outdated
| /// These simulations are unofficial and are known to not necessarily match sightings | ||
| /// on the ground. Unless you know otherwise for sure, instead of this variant, use | ||
| /// [`UmmAlQura`], which uses the results of KACST's Mecca-based calculations. | ||
| /// These rules use the relative positions of the Earth, moon, and sun to predict the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /// These rules use the relative positions of the Earth, moon, and sun to predict the | |
| /// These rules use astronomical calculations to predict the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It says "astronomical calculations" on the previous line, though...
components/calendar/src/cal/hijri.rs
Outdated
| /// These rules can form the basis of a custom [`Rules`] implementation that includes data | ||
| /// based on human sightings. As discussed in the [`Hijri`] documentation, using these | ||
| /// rules without allowing for adjustments will produce dates that are only approximations | ||
| /// of the ground truth. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /// of the ground truth. | |
| /// of reality. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer "ground truth" over "reality" because I really do mean the ground truth, defined by governments. The ground truth varies from region to region. There is no single "reality".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then please state what the "ground truth" is. we've been using this term a lot internally but it's not clear for clients what it means (and it's not even clear to me, because apparently it's not reality). is it the moon as seen by mathematics? is it the moon seen by some dude? is it the moon announced by some authority?
also in other places
components/calendar/src/cal/hijri.rs
Outdated
| /// | ||
| /// These rules can form the basis of a custom [`Rules`] implementation that includes data | ||
| /// based on human sightings. As discussed in the [`Hijri`] documentation, using these | ||
| /// rules without allowing for adjustments will produce dates that are only approximations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does "approximation" mean in terms of a calendar? the dates are either correct or incorrect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dates are correct according to the Shaukat criterion. Those dates often match what governments observe, but not always; they are therefore an approximation of ground truth.
Part of the objective of this PR is to be more clear about what these rules are, because I think we haven't really understood what they are. We've been quick to dismiss HijriSimulated as "inaccurate: never use", without explaining what it actually does and how to use it correctly. If you start by understanding "these rules are an approximation", then you can use them correctly. I went back and re-read Reingold and other sources when writing these docs.
components/calendar/src/cal/hijri.rs
Outdated
| /// | ||
| /// # Crescent moon visibility | ||
| /// | ||
| /// Islam says that months begin when an observer first sees the crescent moon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Islam doesn't "say" anything. According to Islam...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
components/calendar/src/cal/hijri.rs
Outdated
| /// | ||
| /// For centuries, astronomers have been developing criteria for predicting crescent moon | ||
| /// visibility. However, in most regions, atmospheric phenomena can impact visibility, making | ||
| /// such predictions only an approximation of ground truth. Hijri calendar applications should |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /// such predictions only an approximation of ground truth. Hijri calendar applications should | |
| /// such predictions inaccurate. Hijri calendar applications should |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to say "approximation of ground truth".
components/calendar/src/cal/hijri.rs
Outdated
| /// such predictions only an approximation of ground truth. Hijri calendar applications should | ||
| /// therefore include the ability to specify a crescent sighting adjustment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in civil contexts, which is what our calendars are targeting, observational calendars are the odd ones out, not UAQ/tabular.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think any countries use Tabular Islamic. It is used by a few small sects of Islam.
Most countries use observational calendars. Saudi Arabia and I think Turkey publish predictions that match their observations.
components/calendar/src/cal/hijri.rs
Outdated
| /// If you don't have a way to inject human sighting adjustments, you should probably use | ||
| /// [`UmmAlQura`], which uses the results of KACST's Mecca-based calculations and matches | ||
| /// ground truth in Saudi Arabia. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this recommendation is correct. You use UAQ if you need UAQ, tabular if you need tabular, and observational if you need observational (whenever that is). If you need observational and don't have sightings, UAQ is not an alternative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to remove this whole paragraph
| /// | ||
| /// According to Islam, months begin when an observer first sees the crescent moon. | ||
| /// | ||
| /// For centuries, astronomers have been developing criteria for predicting crescent moon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we shouldn't land this change. This change pushes the docs even further to the direction of suggesting that modeling religious Hijri calendar use by approximation is something that internationalized software should do and that ICU4X is suited for doing. I think we don't have evidence that this is what we should be suggesting to app developers who delegate i18n expertise to ICU4X.
I think we should have the discussions we are having in parallel to this pull request about scope. I think we shouldn't be leaning into ways to rationalize keeping shipping SimulatedHijri, when it continues to look to me that we have it because it's sunk-cost code that's appealing to programmers even though we haven't seen a presentation of strong evidence of any particular instantiation of SimulatedHijri addressing substantial user needs. The Cairo parameter came about because it was an example parameter. As for the Mecca parameter, I think we continue to lack a strong case for providing an unofficial Mecca-reference simulation when we provide an official Mecca-reference simulation.
My preference would be to steer the docs in the direction of the following:
- Umm al-Qura is a civil-use calendar that is a Mecca-reference simulation calculated by KACST (with clarity about the data range and fallback).
- Considering the user feedback to Google that resulted in CLDR no longer attesting Umm al-Qura as the primary calendar for Saudi Arabia, we shouldn't be characterizing the current usage level or usage situations of Umm al-Qura unless we actually know.
- AFAICT
islamic-civilis in CLDR, because IBM picked it for reasons that we don't have visibility into, and now it also serves as ICU4C-compatible out-of-KACST-range fallback for Umm al-Qura, andislamic-tblais CLDR because Microsoft picked it for reasons we don't have visibility into (I think we shouldn't retell the stated story without commentary, and commentary doesn't belong in our docs, so I think we shouldn't be telling the stated story). - Tabular Hijri is a centuries old well-known concept, but we shouldn't be suggesting a particular use case for app developers unless we actually know, considering that IBM and Microsoft software compat rather than a more specific use case seems to be why they are included.
- Various sources claim that the leap year rule that both
islamic-tblaandislamic-civiluse is the most common one. It's unclear to me to what extent these sources are repeating each other, but it's likely OK for us to repeat the claim. It's easy to believe that this is why both IBM and Microsoft picked this leap year cycle, but that's a guess.
robertbastian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
components/calendar/src/cal/hijri.rs
Outdated
| /// These rules can form the basis of a custom [`Rules`] implementation that includes data | ||
| /// based on human sightings. As discussed in the [`Hijri`] documentation, using these | ||
| /// rules without allowing for adjustments will produce dates that are only approximations | ||
| /// of the ground truth. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then please state what the "ground truth" is. we've been using this term a lot internally but it's not clear for clients what it means (and it's not even clear to me, because apparently it's not reality). is it the moon as seen by mathematics? is it the moon seen by some dude? is it the moon announced by some authority?
also in other places
components/calendar/src/cal/hijri.rs
Outdated
| /// [`UmmAlQura`], which uses the results of KACST's Mecca-based calculations. | ||
| /// These rules use the relative positions of the Earth, moon, and sun to predict the | ||
| /// exact moment of a new moon, and then apply a criterion proposed by S. K. Shaukat in | ||
| /// 1996 to determine crescent moon visibility. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
linking to an undocumented method is not a citation
| /// As floating point arithmetic degenerates for far-away dates, this falls back to | ||
| /// the tabular calendar at some point. | ||
| /// These rules are based on calculations of the Earth, moon, and sun along with the | ||
| /// Shaukat criterion to determine crescent moon visibility.[^1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change removes the remark "These simulations are unofficial and are known to not necessarily match sightings on the ground." and instead name drops "Shaukat criterion".
To a reader unfamiliar with the topic who is delegating to ICU4X for i18n expertise, this gives appearance of more gravitas (than before this PR) to this variant, which isn't the direction that we should be going in as long as we don't have evidence that this unofficial simulation serves user needs better than sticking to KACST's calculation that has official status.
|
I split this into a bunch of smaller PRs because I think there are parts of this that have consensus and other parts that don't. There is still some language that I need to migrate but I'll wait until the first batch merges. |
Follow-up to #7301
I know we are in disagreement about whether this calendar should exist in the first place. I am opening this PR to improve the documentation of the status quo. This is not an invitation to argue about what the calendar should be; it is an opportunity to be more clear about what we currently ship. Thank you.