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

Editorial: refactor remainder AOs #2911

Merged
merged 1 commit into from
Sep 29, 2022
Merged

Editorial: refactor remainder AOs #2911

merged 1 commit into from
Sep 29, 2022

Conversation

michaelficarra
Copy link
Member

Extracted from #2901. Makes the wording consistent and clearly separates the aliases being declared from the ones being referenced.

Copy link
Contributor

@bakkot bakkot left a comment

Choose a reason for hiding this comment

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

Works for me.

@jmdyck
Copy link
Collaborator

jmdyck commented Sep 20, 2022

There's no need to define _r_ and _q_ 'simultaneously' though. So it'd be simpler to just split each into 2 steps, roughly:

Let _q_ be the integer/BigInt ....
Let _r_ be _n_ - (_d_ x _q_). 

@jmdyck
Copy link
Collaborator

jmdyck commented Sep 20, 2022

(Also, I don't think there's a good reason for the differences in how _q_ is defined.)

@jmdyck
Copy link
Collaborator

jmdyck commented Sep 20, 2022

Since 5.2.5 defines floor(), I think you could say:

Let _quotient_ be ℝ(_n_) / ℝ(_d_).
Let _q_ be the mathematical value whose sign is the sign of _quotient_
   and whose magnitude is floor(abs(_quotient_)).

If you want, you could add a Note-step saying how the sign of _quotient_ relates to the signs of _n_ and _d_.

@bakkot bakkot added the editor call to be discussed in the next editor call label Sep 27, 2022
@michaelficarra michaelficarra force-pushed the relation-introductions branch 2 times, most recently from 81ae934 to 2d69aac Compare September 27, 2022 23:48
@michaelficarra
Copy link
Member Author

Thanks @jmdyck. I accepted your suggestion.

@michaelficarra michaelficarra changed the title Editorial: mention each of the aliases being defined in a relation Editorial: refactor remainder AOs Sep 27, 2022
Copy link
Contributor

@syg syg left a comment

Choose a reason for hiding this comment

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

So I'm fine with this landing, and in particular I think the BigInt reformulation is a strictly clearer wording. That said I prefer this not be precedent-setting for requiring an alias for all intermediates. I want us to be still able to judiciously use where clauses where clear.

@michaelficarra michaelficarra added editor call to be discussed in the next editor call ready to merge Editors believe this PR needs no further reviews, and is ready to land. and removed editor call to be discussed in the next editor call labels Sep 28, 2022
@ljharb ljharb force-pushed the relation-introductions branch from 2d69aac to d3e5d6b Compare September 29, 2022 05:51
@ljharb ljharb merged commit d3e5d6b into main Sep 29, 2022
@ljharb ljharb deleted the relation-introductions branch September 29, 2022 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial change ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants