bugGNU Octave - Bugs: bug #34936, scatter errors after completion...

 
 

bug #34936: scatter errors after completion when more than 100 points

Submitter:  Alejandro Dubrovsky <alito>
Submitted:  Tue 29 Nov 2011 03:31:23 PM UTC
   
 
Category:  Plotting Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  bpabbott
Originator Name:  Open/Closed:  * Closed
Release:  * 3.4.3 Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 20 Dec 2011 02:33:29 PM UTC, comment #22: 

Tested with Octave 3.4.2 on Fedora 15.

Works perfectly!

Thank You Ben!

Merry Christmas!

(file #24634)

Antti Andreimann <anttix>
Thu 08 Dec 2011 01:04:37 AM UTC, comment #21: 

I pushed the change.

http://hg.savannah.gnu.org/hgweb/octave/rev/431c6b00653d

I'll close this item. If some problem persists, this can be reopened.

Ben Abbott <bpabbott>
Group Member
Wed 07 Dec 2011 11:50:57 PM UTC, comment #20: 

I've attached a changset that fixes the problems (as understand them).

The _scatter_() function behaves differently for gnuplot. I'm thinking that gnuplot can't render polygons with different colors at each vertex.

The changeset includes new demos. After applying the patch, the demos can be run by


demo ("scatter", 6:7)


(file #24544)

Ben Abbott <bpabbott>
Group Member
Mon 05 Dec 2011 07:35:02 PM UTC, comment #19: 

I can take a look, but it will be a few days before I have the time.

Ben Abbott <bpabbott>
Group Member
Mon 05 Dec 2011 07:19:23 PM UTC, comment #18: 

Yes, Ben.

Except perhaps my fix is wrong, because it tries to avoid lots of patch objects. This seems to work for fltk, but not for gnuplot. Would you be able to look into that?

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Mon 05 Dec 2011 07:14:38 PM UTC, comment #17: 

Jordi,

Regarding the specific fix which Antti pointed to. If I'm reading the sources correctly, an equivalent fix has already be made, correct?


Ben Abbott <bpabbott>
Group Member
Mon 05 Dec 2011 07:00:38 PM UTC, comment #16: 

Just to make it clear, this is a case on which your fix breaks:


 n = 500;
 x = rand(n,1);
 y = rand(n,1);
 idx = ceil(rand(n,1)*3);
 colors = eye(3)(idx, :);
 scatter(x,y,15, colors, "filled")


Jordi Gutiérrez Hermoso <jordigh>
Group Member
Mon 05 Dec 2011 06:55:51 PM UTC, comment #15: 

Antti, your bugfix doesn't work. It doesn't cover all cases. The whole scatter code needs a lot of work.

You don't have to subscribe to the maintainers' list if you don't want to. You can browse it online. I just wanted to point out that as Rik and I have verified, the simple fix proposed doesn't completely solve the problem.

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Mon 05 Dec 2011 05:36:49 PM UTC, comment #14: 

I do not want to sound impolite, but I am not willing to subscribe to every single newsgroup and website out there just to report a bug.

I already created an account at savannah only to report the bug and took time to do some research and create an isolated test case.

The simple two line fix proposed does no harm and fixes at least some of the shortcomings right now until someone comes forward with a better proposal.

See comment #6.

I hope someone can integrate at least a temporary fix before February so that Octave (and free software in general) does not suffer from bad publicity amongst new users attending the Stanford's ML course. Most of us have never used Matlab or Octave before.

Currently there are two threads discussing this particular bug at Stanford and both of them have over 1k views.

Just my 2 cents.

Antti Andreimann <anttix>
Mon 05 Dec 2011 04:06:27 PM UTC, comment #13: 

Antti, the situation is more complicated than that one-line fix. Please read the rest of the replies in this bug report as well as its continuation in the maintainers' list:

http://octave.1599824.n4.nabble.com/Slowing-scatter-down-for-the-sake-of-Matlab-compatibility-td4158171.html

This bug has a long history. Please continue its discussion in the maintainers' list.

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Mon 05 Dec 2011 11:22:33 AM UTC, comment #12: 

Just to note that the patch proposed earlier by "None" also fixes the error experienced when running my sample code as well as the code included with Stanford's ML-class exercises.

Please consider applying one of the fixes for the next version of Octave. I suspect that due to the fact that Stanford's very popular ML-class will be repeated in February 2012, the number of
new Octave users that are likely to be hit by the bug will increase significantly.

Thank you!

Antti Andreimann <anttix>
Mon 05 Dec 2011 11:09:52 AM UTC, comment #11: 

Experiencing the same here.
OS: Fedora 15
Octave 3.4.2


octave:3> X = rand(120, 2); y = repmat([1:3]', 40, 1);
octave:4> scatter(X(:,1), X(:, 2), [], hsv(4)(y, :));
error: __scatter__: A(I): index out of bounds; value 4 out of bound 3
error: called from:
error:   /usr/share/octave/3.4.2/m/plot/private/__scatter__.m at line 201, column 13
error:   /usr/share/octave/3.4.2/m/plot/scatter.m at line 71, column 11


A fix was posted to Stanford's ml-class.org forums by Timo Partanen http://www.ml-class.org/course/qna/view?id=4732

The attached patch implements it.

Please apply.

Thank you!

(file #24537)

Antti Andreimann <anttix>
Sun 04 Dec 2011 08:42:43 PM UTC, comment #10: 

As a matter of preference, I'm uncomfortable breaking with compatibility.

Personally, I'd prefer to remain compatible when possible.

Beyond, personal preference, from Matlab it is possible to select specific points of the scatter plot using the mouse. This is done using brush().

http://www.mathworks.com/help/techdoc/ref/brush.html


Ben Abbott <bpabbott>
Group Member
Sun 04 Dec 2011 07:48:45 PM UTC, comment #9: 

Is that compatibility we need to keep? Doing 1000 patches is awfully slow.

The history does go back earlier than that. To tell hg to give you the history of a file across renames/moves, do


hg log -f filename


Jordi Gutiérrez Hermoso <jordigh>
Group Member
Sun 04 Dec 2011 07:43:35 PM UTC, comment #8: 

Regarding the number of patches, ML produces one patch for each point. For Rik's example;


x = rand(101,1);
y = rand(101,1);
colours = [ ones(101,1) zeros(101,1) zeros(101, 1)];
colours(1,:) = [0 0 1];
hg = scatter(x,y, 15, colours);
hp = findall (hg, 'type', 'patch');


The number of patch objects for, ML R2011b, is 101.

I recall we had discussed this a few years back (I think David Bateman noticed this). I also recall this had been fixed.  Unfortunately, hg's log doesn't go back far enough.

The earliest hg log entry is

http://hg.savannah.gnu.org/hgweb/octave/rev/2f435ed48143

Conveniently, it looks like this change reintroduced the incompatibility.

For the curent sources, I think ML compatibility may be restored by replacing remove the "else" part of the if-block starting with


  if (numel (x) <= 100)


and unconditionally using the code associated with a logical true.



Ben Abbott <bpabbott>
Group Member
Sun 04 Dec 2011 04:08:02 PM UTC, comment #7: 

I have found what the problem with colours was:

    http://hg.savannah.gnu.org/hgweb/octave/rev/7ab497513c1a

When someone confirms that this really fixes the problem, we can close this bug report.

The _scatter_.m code seems to be creating way more patch objects than are necessary, even in the supposedly optimised case for n >= 100 points. It appears that the _go_patch_ function can accept vector arguments for all properties except markersize, so it seems to me that there should be one patch object per size, not per colour. I will reorganise the code later after this fix is confirmed.

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Sun 04 Dec 2011 03:31:26 PM UTC, comment #6: 

After some testing (thanks, Joanna Cheng), it appears that this patch doesn't fix the bug (but imo is an improvement, and should stay). In gnuplot the _go_patch_ object errors out with this call and in OpenGL the colours are wrong. This may be related to another observed when the colours of bars are wrong in a histogram (I can't find the reference to that bug right now, or if it has been filed).

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Sun 04 Dec 2011 03:02:10 AM UTC, comment #5: 

Fixed in this changeset, almost certainly:

    http://hg.savannah.gnu.org/hgweb/octave/rev/6be49527e4cf

You can grab the fixed file here, in case you need a quick backport of the fix to 3.4.3:

    http://hg.savannah.gnu.org/hgweb/octave/raw-file/6be49527e4cf/scripts/plot/private/__scatter__.m

HTH

Jordi Gutiérrez Hermoso <jordigh>
Group Member
Wed 30 Nov 2011 05:53:39 PM UTC, comment #4: 

Unfortunately, a true fix is going to be more involved.  The patch submitted works for the case presented, but not for a second simple case where there is more than one color.


x = rand(101,1);
y = rand(101,1);
colours = [ ones(101,1) zeros(101,1) zeros(101, 1)];
colours(1,:) = [0 0 1];  # different color for first element
scatter(x,y, 15, colours)
error: vertical dimensions mismatch (1x1 vs 1x2)
error: evaluating argument list element number 1
error: evaluating argument list element number 2
error: called from:
error:   /home/rik/wip/Projects_Mine/octave-dev/scripts/plot/private/__scatter__.m at line 269, column 9
error:   /home/rik/wip/Projects_Mine/octave-dev/scripts/plot/private/__scatter__.m at line 198, column 18
error:   /home/rik/wip/Projects_Mine/octave-dev/scripts/plot/scatter.m at line 71, column 11


Rik <rik5>
Group administrator
Wed 30 Nov 2011 10:48:44 AM UTC, comment #3: 

And thank you, anonymous patch submitter

Alejandro Dubrovsky <alito>
Wed 30 Nov 2011 10:46:35 AM UTC, comment #2: 

Confirmed that the patch fixes the issue I mentioned.

Alejandro Dubrovsky <alito>
Tue 29 Nov 2011 06:35:03 PM UTC, comment #1: 

Confirmed on the development branch as well. 

In the private function _scatter_.m there is a conditional test on the number of points.  If the number is <= 100 one body of code is used.  If the number of points is greater than 100 a different body of code is used and it is really dysfunctional.

Rik <rik5>
Group administrator
Tue 29 Nov 2011 03:31:23 PM UTC, original submission:  

When specifying the colours in R G B mode, scatter errors when more than 100 points are specified.

For example, this code errors after displaying the right diagram:


x = rand(101,1);
y = rand(101,1);
colours = [ ones(101,1) zeros(101,1) zeros(101, 1)];
scatter(x,y, 15, colours)


with error


error: __scatter__: A(I): index out of bounds; value 2 out of bound 1
error: called from:
error:   /usr/share/octave/3.4.3/m/plot/private/__scatter__.m at line 201, column 13
error:   /usr/share/octave/3.4.3/m/plot/scatter.m at line 71, column 11


Alejandro Dubrovsky <alito>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #24634:  octave-3.4.2-scatter-fix.patch added by anttix (6KiB - text/x-patch - Ben's fix backported to 3.4.2)
file #24544:  changeset.patch added by bpabbott (8KiB - application/octet-stream)
file #24504:  scatter-bug.patch added by None (667B - application/octet-stream - Attached a patch against 3.4.3 that should fix at least one problem with this code.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by anttix (Updated the item)
  • -email is unavailable- added by joannac
  • -email is unavailable- added by bpabbott (Posted a comment)
  • -email is unavailable- added by jordigh (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by alito (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 14 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-12-20 anttix Attached File- Added octave-3.4.2-scatter-fix.patch, #24634
    2011-12-08 bpabbott StatusPatch Submitted Fixed
        Assigned toNone bpabbott
        Open/ClosedOpen Closed
    2011-12-07 bpabbott Attached File- Added changeset.patch, #24544
    2011-12-05 anttix Attached File- Added octave-scatter-color-map-fix.patch, #24537
    2011-12-05 joannac Carbon-Copy- Added -email is unavailable-
    2011-12-04 bpabbott Item GroupNone Matlab Compatibility
    2011-12-04 jordigh StatusFixed Patch Submitted
        Open/ClosedClosed Open
    2011-12-04 jordigh StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2011-11-30 None Attached File- Added scatter-bug.patch, #24504
    2011-11-29 rik5 StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code