bugGNU Octave - Bugs: bug #53214, area.m needs update to implement...

 
 

bug #53214: area.m needs update to implement ShowBaseline and other properties

Submitter:  Marshall <marsian>
Submitted:  Sat 24 Feb 2018 01:44:54 AM UTC
   
 
Category:  Plotting Severity:  1 - Wish
Priority:  3 - Low Item Group:  Feature Request
Status:  In Progress Assigned to:  None
Originator Name:  Marsian Open/Closed:  * Open
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 14 Sep 2023 04:22:58 PM UTC, comment #16: 
Nicholas Jankowski <nrjank>
Group Member
Thu 14 Sep 2023 04:10:32 PM UTC, comment #15: 

hmmm, good point on the failed test. the %!test line was supposed to include the bug number. not sure how I missed that. I'll revert the change and attach a cleaned up the patch here for later consideration.

Nicholas Jankowski <nrjank>
Group Member
Thu 14 Sep 2023 03:48:54 PM UTC, comment #14: 

Is that really what we'd want?
This breaks use cases like:

area(1.5:10.5, rand(10))


In Matlab, this has x limits from 0 to 12.

That's the same in Octave before this change.

Ideally, we'd need to implement the special treatment of a lower limit of 1 somewhere deeper in the code.

Additionally, what does `%!test <*>` mean?

Can we revert this change until we have a better solution?

Markus Mützel <mmuetzel>
Group administrator
Thu 14 Sep 2023 01:37:57 PM UTC, comment #13: 

pushed a patch to default that simply adds Rik's suggested two lines to area.m where suggested, and also adds a BIST checking xlim for a couple cases like those shown in comment #8 and comment #9.  also adds a fixme note that the change might be better done inside graphics.cc rather than corrected after the fact.
http://hg.savannah.gnu.org/hgweb/octave/rev/7997e785db0f

I was also going to make the same changes to plot.m, which has the same effect of fixing the limits shown in comment #9, but the change causes 3 failures and 1 regression in the test suite, so I didn't push the change.

Two fails and a regression are in graphics.cc:


**** test
 hf = figure ("visible", "off");
 unwind_protect
   hax = axes ("parent", hf);
   plot (0, pi);
   assert (get (hax, "xlim"), [-1, 1]);
   assert (get (hax, "xlimmode"), "auto");
   assert (get (hax, "ylim"), [2.8, 3.5], 2*eps);
   assert (get (hax, "ylimmode"), "auto");
   set (hax, "xlim", [1, 1], "ylim", [0, 0]);
   assert (get (hax, "xlim"), [0.9, 1.1]);
   assert (get (hax, "xlimmode"), "manual");
   assert (get (hax, "ylim"), [0, 1]);
   assert (get (hax, "ylimmode"), "manual");
   set (hax, "xlim", [-Inf, Inf], "ylim", [-Inf, Inf]);
   ## Matlab does not update the properties
   assert (get (hax, "xlim"), [0, 1]);
   assert (get (hax, "ylim"), [0.9, 1.1]*pi, 2*eps);
 unwind_protect_cleanup
   delete (hf);
 end_unwind_protect
!!!!! test failed
ASSERT errors for:  assert (get (hax, "ylim"),[2.8, 3.5],2 * eps)

  Location  |  Observed  |  Expected  |  Reason
    (1)         2.8274        2.8        Abs err 0.027433 exceeds tol 4.4409e-16 by 0.03
    (2)         3.4558        3.5        Abs err 0.044248 exceeds tol 4.4409e-16 by 0.04
***** test
 hf = figure ("visible", "off");
 unwind_protect
   hax = axes ("parent", hf);
   plot ([0, exp(1)], [-pi, 3]);
   assert (get (hax, "xlim"), [0, 3]);
   assert (get (hax, "xlimmode"), "auto");
   assert (get (hax, "ylim"), [-4, 3]);
   assert (get (hax, "ylimmode"), "auto");
   set (hax, "xlim", [-Inf, Inf], "ylim", [-Inf, Inf]);
   ## Matlab does not update the properties but uses tight limits on screen
   assert (get (hax, "xlim"), [0, exp(1)]);
   assert (get (hax, "xlimmode"), "manual");
   assert (get (hax, "ylim"), [-pi, 3]);
   assert (get (hax, "ylimmode"), "manual");
 unwind_protect_cleanup
   delete (hf);
 end_unwind_protect
!!!!! test failed
ASSERT errors for:  assert (get (hax, "xlim"),[0, 3])

  Location  |  Observed  |  Expected  |  Reason
    (2)         2.7183         3         Abs err 0.28172 exceeds tol 0 by 0.3
***** test <*63624>
 hf = figure ("visible", "off");
 unwind_protect
   hax = axes ("parent", hf);
   plot (hax, [1, 201], [0, 1]);
   assert (get (hax, "xtick"), 0:50:250);
   axis (hax, "tight");
   assert (get (hax, "xtick"), 50:50:200);
 unwind_protect_cleanup
   delete (hf);
 end_unwind_protect
!!!!! regression: https://octave.org/testfailure/?63624
ASSERT errors for:  assert (get (hax, "xtick"),0:50:250)


The third failure is in linkaxes

>>>>> processing /home/nrjank/source/octave/scripts/plot/util/linkaxes.m
***** test
 hf1 = figure ("visible", "off");
 hax1 = axes ();
 plot (1:10);
 hf2 = figure ("visible", "off");
 hax2 = axes ();
 plot (10:-1:1, "-*g");
 hf3 = figure ("visible", "off");
 hax3 = axes ();
 plot (1:10:100, "-xb");
  unwind_protect
   linkaxes ([hax1, hax2, hax3]);
   ## Test initial values taken from first object in list
   assert (xlim (hax3), [0 10]);
   assert (ylim (hax3), [0 10]);
   ## Test linking
   xlim (hax2, [2 8]);
   assert (xlim (hax1), [2 8]);
   assert (xlim (hax3), [2 8]);
   ylim (hax3, "auto");
   assert (ylim (hax1), [0 100]);
   assert (ylim (hax2), [0 100]);
   ## Test re-linking removes old link
   linkaxes ([hax1, hax2]);
   ylim (hax3, [0 50]);
   assert (ylim (hax1), [0 100]);
   assert (ylim (hax2), [0 100]);
   xlim (hax1, [0 4]);
   assert (xlim (hax2), [0 4]);
   ## Test linking of remaining objects after deletion of one object
   linkaxes ([hax1, hax2, hax3]);
   xlim (hax2, [0 1]);
   assert (xlim (hax1), [0 1]);
   assert (xlim (hax3), [0 1]);
   delete (hax2);
   xlim (hax3, [0 2]);
   assert (xlim (hax1), [0 2]);
   ## Test deletion of link
   linkaxes ([hax1, hax3], "off");
   xlim (hax3, [0 3]);
   assert (xlim (hax1), [0 2]);
  unwind_protect_cleanup
   close ([hf1 hf2 hf3]);
  end_unwind_protect
!!!!! test failed
ASSERT errors for:  assert (xlim (hax3),[0, 10])

  Location  |  Observed  |  Expected  |  Reason
    (1)           1            0         Abs err 1 exceeds tol 0 by 1


that last one seems like it could just be arbitrarily changed to match the new behavior, but not sure about the ones in graphics.cc.

If we want to leave plot.m alone, then this could just be Ready for Test, but marking as In Progress for now.

Nicholas Jankowski <nrjank>
Group Member
Sun 21 Nov 2021 11:47:26 PM UTC, comment #12: 

For the x limits, a possible workaround is to add the following code under the "if (! ishold ())" block.


axis (hax, "tight")
set (hax, "ylimmode", "auto")


Rik <rik5>
Group administrator
Thu 26 Aug 2021 02:01:16 AM UTC, comment #11: 

forgot to add - the main bug item has been fixed.  it would be nice to have the xlim match, but this could be closed and that could be made a new bug report, as it applies to more than just area.m.  alternatively this bug report could be retitled appropriately with the item group changed to Matlab Compatibility.

Nicholas Jankowski <nrjank>
Group Member
Thu 26 Aug 2021 01:58:55 AM UTC, comment #10: 

in area.m, then patch.m, the axis limits are set in the call to _go_patch_ (line 176)

in plot.m, then _plt_.m, the axis limits are set in the call to _go_line_ (line 435)

both are compiled functions in  libinterp/corefcn/graphics.cc, both are passed x-data consisting of [1:10].  so both are internally deciding to set the left limit to 0 rather than the lowest value. As before patch hands off the job it has set the xlim to [-1 1], it seems to be a case of resetting axes min to 0, rather than just neglecting to update it to x_data_min.

not set up to step through compiled functions to verify, but taking a quick peek at graphics.cc, both call GO_BODY(patch/line).  that's where i get lost in the 14k line file, so someone familiar will maybe need to see where it's not setting the left xlim to be 1.  

I would suggest both plot and area get a compatibility test added such as:


## test default plot limits
%!test
%! h = plot(rand(10))
%! assert(xlim(gca),[1 10])


Nicholas Jankowski <nrjank>
Group Member
Thu 26 Aug 2021 12:33:34 AM UTC, comment #9: 

as noted in comment #6, the origin incompatibility also appears in plot, which makes sense as area is just plot with area below the curve filled in. now, are plot and area just doing the same thing, or do they pull from the same code. will peek.




Nicholas Jankowski <nrjank>
Group Member
Thu 26 Aug 2021 12:24:42 AM UTC, comment #8: 

comparing 6.3.0 with matlab, outputs look identical except for the axes limits.




Nicholas Jankowski <nrjank>
Group Member
Thu 26 Aug 2021 12:04:01 AM UTC, comment #7: 

Using 6.1.0

Following the code from comment #2


area(rand(10),'facealpha',0.5)
hold on
area(rand(10),'facealpha',0.5)
hold off


No errors are produced.
The figure xlim is still [0,10]




Anonymous
Wed 28 Feb 2018 01:14:14 AM UTC, comment #6: 

Yes, Matlab sets x limits at [1,10]. See the attached figure. It also happens with plot...


Marshall <marsian>
Tue 27 Feb 2018 10:04:22 PM UTC, comment #5: 

Since you have access to Matlab, what does


area (rand (10))


look like?  Octave sets the x limits at [0, 10] which seems silly.  It looks like it would be better to set the limits at [1, 10] and closely hug the data.

Rik <rik5>
Group administrator
Tue 27 Feb 2018 09:56:52 PM UTC, comment #4: 

This works great for me!

Marshall <marsian>
Mon 26 Feb 2018 05:48:02 PM UTC, comment #3: 

The area.m function has not been updated now that patch implements facealpha.  In fact, there are several unimplemented properties.

I see that BaseLine and ShowBaseline are unimplemented, although BaseValue is.  Also, XDataSource, YDataSource, and XDataMode are likewise not implemented.

This was a pretty easy fix so I added support for EdgeAlpha and FaceAlpha in this cset (http://hg.savannah.gnu.org/hgweb/octave/rev/1cacfebaed3e).

Rik <rik5>
Group administrator
Mon 26 Feb 2018 03:30:12 PM UTC, comment #2: 

+verbatm+

>> area(rand(10),'facealpha',0.5)

hold on
area(rand(10),'facealpha',0.5)
hold off
error: set: unknown hggroup property facealpha
error: called from
    area>__area__ at line 188 column 7
    area at line 117 column 10
-verbatim-

It fails on the first call above. If working properly, it would continue to the second area call and would appear with some transparency. I know transparency has been implemented with some of the other functions now in dev (patch, surface, ...), so allowing transparency for area would be ideal. If transparency can't be added, at the very least it shouldn't error out.

Marshall <marsian>
Sun 25 Feb 2018 05:32:41 PM UTC, comment #1: 

Which commands are you running to produce the error? Which error are you seeing exactly?

I won't be able to look into this during the next two weeks or so. But might have a look at this later if no one else wants to check earlier.

Markus Mützel <mmuetzel>
Group administrator
Sat 24 Feb 2018 01:44:54 AM UTC, original submission:  

Use of the 'area' function with the facealpha property causes an error. At the very least the option shouldn't cause an error for matlab compatibility, but even better would be to implement it.

I'm using a dev version on Win 10 compiled on Feb 1st, and I don't think any changes since then would have fixed this.

Marshall <marsian>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #55135:  area_limits_patch-bug53214.diff added by nrjank (2KiB - application/octet-stream - preliminary patch to set area limits to tight)
file #51820:  plot_origin_compare.png added by nrjank (163KiB - image/png)
file #51819:  facealpha_compare.png added by nrjank (219KiB - image/png)
file #51818:  rand.png added by None (134KiB - image/png)
file #43410:  area10.PNG added by marsian (23KiB - image/png)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by nrjank (Updated the item)
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by marsian (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 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-09-14 nrjank Attached File- Added area_limits_patch-bug53214.diff, #55135
    2023-09-14 nrjank StatusConfirmed In Progress
    2021-08-26 nrjank Attached File- Added plot_origin_compare.png, #51820
    2021-08-26 nrjank Attached File- Added facealpha_compare.png, #51819
    2021-08-26 None Attached File- Added rand.png, #51818
    2019-01-12 jwe Severity3 - Normal 1 - Wish
        Priority5 - Normal 3 - Low
        Item GroupMatlab Compatibility Feature Request
    2018-02-28 marsian Attached File- Added area10.PNG, #43410
    2018-02-26 rik5 StatusNeed Info Confirmed
        Summaryfacealpha property causes error when used with area() area.m needs update to implement ShowBaseline and other properties
    2018-02-25 mmuetzel StatusNone Need Info

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code