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

Rational.REDUCE-ME has a data race #6069

Closed
p6rt opened this issue Feb 13, 2017 · 9 comments
Closed

Rational.REDUCE-ME has a data race #6069

p6rt opened this issue Feb 13, 2017 · 9 comments
Labels
Bug

Comments

@p6rt
Copy link

@p6rt p6rt commented Feb 13, 2017

Migrated from rt.perl.org#130774 (status was 'resolved')

Searchable as RT130774$

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 13, 2017

From @mscha

It's fairly rare to encounter a Rat that isn't normalized, but if you do
get one, norm() doesn't normalize the numerator/denominator.
nude() does, though, and actually changes the Rat (which is supposed to
be immutable).

Example​:

my $f = 1/13² + 1/26² + 1/39² + 1/78²
0.008218
say $f.numerator, '/', $f.denominator;
50/6084
my $g = $f.norm;
0.008218
say $g.numerator, '/', $g.denominator;
50/6084
say $f.nude.join('/');
25/3042
say $f.numerator, '/', $f.denominator;
25/3042
say $g.numerator, '/', $g.denominator;
25/3042

Even stranger, $g is now normalized as well.

This is on Rakudo Star 2017.01, 64-bit Linux.

% perl6 --version
This is Rakudo version 2017.01 built on MoarVM version 2017.01
implementing Perl 6.c.

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 13, 2017

From @mscha

Actually, it's not as rare as I thought​: the same thing happens when you do​:

my $f = 1/6 + 1/6

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 13, 2017

From @zoffixznet

The avoidance of reduction is for optimization purposes.

The .norm stuff is fixed now in rakudo/rakudo@aac9efc and tested in Raku/roast@7d0daf5

However, there's a data-race in Rational.REDUCE-ME and it needs to go​: https://irclog.perlgeek.de/perl6-dev/2017-02-13#i_14093035

Renaming the ticket to that.

@p6rt
Copy link
Author

@p6rt p6rt commented Feb 13, 2017

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

@p6rt p6rt commented Mar 6, 2017

From @zoffixznet

Another issue the .REDUCE-ME thing causes​:

mscha │ m​: say (1/2+1/2, 2/2).unique;
+camelia │ rakudo-moar 9da50e​: OUTPUT​: «(1 1)␤»

infix​:<+> does not call .REDUCE-ME so the rats are different in this case

@p6rt
Copy link
Author

@p6rt p6rt commented Aug 27, 2017

From @skids

On Mon, 06 Mar 2017 05​:48​:07 -0800, cpan@​zoffix.com wrote​:

Another issue the .REDUCE-ME thing causes​:

mscha │ m​: say (1/2+1/2, 2/2).unique;
+camelia │ rakudo-moar 9da50e​: OUTPUT​: «(1 1)␤»

infix​:<+> does not call .REDUCE-ME so the rats are different in this case

I could see where a mutable Rational could be useful. Should we parameterize that
role in-core to provide immutable (for Rat and FatRat) and mutable versions for module
authors? Or should we kick all mutable functionality out into module space and
have module authors provide their own role variant... maybe just adding a single
method to Rat/FatRat to allow introspection that they are immutable?

Naturally any mutable Rational would come with the caveat that it is not threadsafe.

Also, what behavior are we looking for Rat -- just "immutabilty" or "value-type"?

@p6rt
Copy link
Author

@p6rt p6rt commented Apr 30, 2018

From @AlexDaniel

The issue with .unique was resolved in these commits

Rakudo​: rakudo/rakudo@8cd70d1
Roast​: Raku/roast@ad38801
Roast​: Raku/roast@032ce8d

On 2017-03-06 05​:48​:07, cpan@​zoffix.com wrote​:

Another issue the .REDUCE-ME thing causes​:

mscha │ m​: say (1/2+1/2, 2/2).unique;
+camelia │ rakudo-moar 9da50e​: OUTPUT​: «(1 1)␤»

infix​:<+> does not call .REDUCE-ME so the rats are different in this case

@p6rt
Copy link
Author

@p6rt p6rt commented May 20, 2018

From @zoffixznet

Data race is now fixed (in a post release branch)​:

Rakudo fix​: rakudo/rakudo@6dd2058
Test​: Raku/roast@1d10e9d

@p6rt
Copy link
Author

@p6rt p6rt commented May 20, 2018

@zoffixznet - Status changed from 'open' to 'resolved'

@p6rt p6rt closed this May 20, 2018
@p6rt p6rt added the Bug label Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.