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

[rand.util.canonical] Convert normative duplication into a note. #3820

Merged
merged 1 commit into from Mar 5, 2020

Conversation

zygoloid
Copy link
Member

@zygoloid zygoloid commented Mar 5, 2020

No description provided.

@zygoloid zygoloid requested a review from tkoeppe March 5, 2020 00:02
@tkoeppe
Copy link
Contributor

tkoeppe commented Mar 5, 2020

This seems more economical and less ad-hoc.

@opensdh: would you care to opine?

produced by \tcode{g}
are uniformly distributed,
the instantiation's results
$t_j$, $0 \leq t_j < 1$,
Copy link
Contributor

Choose a reason for hiding this comment

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

This normative text claims something desirable that the algorithm does not satisfy (because the algorithm is buggy: LWG2524). Should we wait to remove it until we have corrected the algorithm?

Copy link
Member Author

@zygoloid zygoloid Mar 5, 2020

Choose a reason for hiding this comment

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

To my reading, the old wording only said that results in [0, 1) are uniformly distributed, and didn't say the results were necessarily in that range (and didn't say anything at all about the results being in [0,1) if g doesn't produce uniformly distributed results). But it does seem ambiguous. I think the new note below is correct, though: S/R^k is in [0, 1), because it's a real number, prior to the conversion to RealType. (Though what "using arithmetic of type RealType" means is anyone's guess.)

Copy link
Contributor

Choose a reason for hiding this comment

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

It reads to me as if the inequality is an aside that's meant to apply to all values, and it's certainly not the case (in reality) that some are more uniformly distributed than others. You're right that it seems to be conditioned on the uniform distribution from g. The new note is plainly correct; I'm just a little uneasy about making the standard unambiguously say something bad (before fixing it to say something good anywhere). I don't really think that anyone is going to reject P0952 because of this pull request; it's just a feeling about proper ordering.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, so how do we proceed? This existing paragraph smells bad, and I want to do something to it. I don't want to promote the aside the 0 <= t_j < 1 into an actual specification over the produced value of type RealType, because I don't think that is required by the current wording; this whole paragraph reads as a non-normative description of the intent of the math specified below. But we are in a pickle, because the intent is correct and the math is wrong.

Perhaps I should just pretend I didn't see this and wait for your paper to fix it :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I am generically in favor of the editors changing the standard as and only as I see fit, yes.

Anyway, you have an approval from the first author of that paper. If you want to be rid of this, go ahead; I have already said I don't predict any actual harm (and implementations already do the bad thing that the rest of the wording specifies). But I'd be marginally happier waiting (the paper won't even conflict with this).

@opensdh
Copy link
Contributor

opensdh commented Mar 5, 2020

This seems more economical and less ad-hoc.

@opensdh: would you care to opine?

I agree that in general this is a good idea.

@tkoeppe
Copy link
Contributor

tkoeppe commented Mar 5, 2020 via email

@zygoloid zygoloid merged commit 63da1a5 into cplusplus:master Mar 5, 2020
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.

None yet

3 participants