Skip to content
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

Use "cohort of" and "quantum of" rather than array indices #156

Merged
merged 9 commits into from
Jun 11, 2024

Conversation

jessealama
Copy link
Collaborator

Also, restructure definitions of "is zero" and "is finite" for Decimal128 values.

Partial fix for #155

@jessealama jessealama requested a review from ptomato June 11, 2024 11:34
@jessealama jessealama linked an issue Jun 11, 2024 that may be closed by this pull request
27 tasks
@jessealama
Copy link
Collaborator Author

(This is just an editorial change.)

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, but as an end goal we should work towards v being alwyas a real number rather than sometimes a decimal and sometimes a real.

Copy link
Collaborator

@ptomato ptomato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To do this, I think we need to have a definition of "the quantum of d" in the introduction. It's currently not there. We also have some inconsistency between "the cohort of d" and "cohort(d)" there.

spec.emu Outdated Show resolved Hide resolved
jessealama and others added 2 commits June 11, 2024 15:23
Co-authored-by: Philip Chimento <pchimento@igalia.com>
@jessealama
Copy link
Collaborator Author

To do this, I think we need to have a definition of "the quantum of d" in the introduction. It's currently not there. We also have some inconsistency between "the cohort of d" and "cohort(d)" there.

Agreed. One difficulty though is that "cohort(d)" is understood by ecmarkup as an invocation of an AO. Currently, "cohort(d)" is just defined in prose. It could be an AO, though.

@ptomato
Copy link
Collaborator

ptomato commented Jun 11, 2024

You could define it the same way as e.g. floor(x) is defined in ecma262 (see the section "Mathematical Operations")

@jessealama
Copy link
Collaborator Author

You could define it the same way as e.g. floor(x) is defined in ecma262 (see the section "Mathematical Operations")

That's a great idea. I've done that now for both cohort and quantum.

@jessealama jessealama merged commit dde15aa into main Jun 11, 2024
@jessealama jessealama deleted the 155-june-spec-review branch June 11, 2024 13:03
@jessealama jessealama mentioned this pull request Jun 11, 2024
27 tasks
@waldemarhorwat
Copy link

This looks good, but as an end goal we should work towards v being alwyas a real number rather than sometimes a decimal and sometimes a real.

In some cases, such as an input to RoundDecimal128, then yes, v should always be a real number.

In other cases, such as the definition of the «v, q» tuple, we have v be a member of the set FiniteDecimalCohorts. Here v is either a nonzero real number or one of two special spec symbols which we happen to write as +0𝔻, -0𝔻.

v is never a Decimal128 value. The 𝔻 suffix just distinguishes these symbols from +0𝔽 and -0𝔽.

In my initial design (prior to the one I published as issue #122) I had v always be a real number, but that caused too much confusion. Due to the need to distinguish ±0, the representation of a Decimal128 became «svq», where the sign s ∈ {-1, 1}, nonnegative magnitude v ∈ {n × 10q | nq ∈ ℤ, 0 ≤ n < 1034, -6176 ≤ q ≤ 6111}, and q is as before. One problem became the constant need to strip the sign when writing these tuples and add it back when reading them. To get the mathematical value one can't just use v; one needs to multiply it by s.

The major problem with that approach was that we need the cohort function throughout the spec but we can't define one that returns a real number for zero — that would coalesce +0 and -0 and produce incorrect results.

The math algorithms already must include special cases for +0 and -0, so the current definition of the «v, q» tuple is much simpler.

@ptomato
Copy link
Collaborator

ptomato commented Jun 12, 2024

It might prevent confusion about whether +0𝔻, -0𝔻 are Decimal values, if we were to write them as spec enums: ~positive-zero~, ~negative-zero~.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

June spec review
4 participants