bugGNU Octave - Bugs: bug #39433, subplot sizes wrong

 
 

bug #39433: subplot sizes wrong

Submitted by:  Michael Godfrey <godfrey>
Submitted on:  Mon 08 Jul 2013 12:22:37 AM UTC  
 
Category: PlottingSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Regression
Status: FixedAssigned to: Ben Abbott <bpabbott>
Originator Name: Michael GodfreyOpen/Closed: Closed
Release: devOperating System: GNU/Linux

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Wed 14 Aug 2013 03:42:40 PM UTC, comment #14:

I'm having second thoughts about changing the spacing. I'd prefer to maintain compatibility.

Also there remain some problems with updating the tightinset. When fixed the subplots will be impacted.

So, I'll close this for now.

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Fri 19 Jul 2013 02:43:06 PM UTC, comment #13:

With the changesets below the should be fixed.

http://hg.savannah.gnu.org/hgweb/octave/rev/350cad34b0f8
http://hg.savannah.gnu.org/hgweb/octave/rev/942d892524b3

Further adjustments are needed to improve (decrease) the subplot spacing. For example the 1st demo has too much space between subplots.

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Tue 16 Jul 2013 08:09:38 PM UTC, comment #12:

Ben,

Not to worry. subplot works fine for me
with "align" set. All that is needed for the
subplot problem as reported in this bug
report is setting "align" as default and
improving the documentation to reflect the
current state.

Do not feel any pressure to do this before all
the other fixes you are working on!

Michael Godfrey <godfrey>
Tue 16 Jul 2013 07:45:07 PM UTC, comment #11:

Michael, I don't want it to look like I'm ignoring you. I have little spare time right now, and am trying to focus my time to work on Octave on fixing the problems with "position", "outerpostion", "looseinset", and "tightinset".

I have made some progress in understanding the problem in the last few days and hope to propose a changeset (or maybe two) that improves things soon.

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Sun 14 Jul 2013 12:27:43 AM UTC, comment #10:

Ben,

The demo will help, but:

I still think that initializing align_axes to true
at line 71 of subplot.m is a good idea in any case.

This is what almost everyone will want. Those who
do not can still turn align off, I think.

Michael Godfrey <godfrey>
Sat 13 Jul 2013 09:13:25 PM UTC, comment #9:

I've added a demo to subplot.m that illustrates the problem with synchronizing the axes' positions.

http://hg.savannah.gnu.org/hgweb/octave/rev/47cc8e3d9183

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Mon 08 Jul 2013 09:28:28 PM UTC, comment #8:

Ben

Looking quickly at subplot.m I notice the following
are initialized right after the function entry:

align_axes = false;
replace_axes = false;
have_position = false;
initial_args_decoded = false;

Later, only for gnuplot, align_axes is set true.

But also,
"position" can be used to set have_position = true
and then later in the code
"align" can be used to set align_axes = true, and
"replace" can be used to set replace_axes = true

These settings seem to be dependent on other argument
processing which is not very clear. So, some care
will be needed in making changes. Also, "replace"
and "position" are not documented, so their effect needs
to be figured out and documented, along with "align".

So, I still think that initializing align_axes to true
at line 71 of subplot.m is a good idea in any case.

Then, all this can be figured out, compared with Matlab,
and documented.

Do you want to do the one line patch?

Michael Godfrey <godfrey>
Mon 08 Jul 2013 09:01:36 PM UTC, comment #7:

Ben,

In the meantime I think it would be good to
make "align" the default.

Then, the obscure differences from Matlab, etc. can
be puzzled over...

Michael Godfrey <godfrey>
Mon 08 Jul 2013 08:16:35 PM UTC, comment #6:

Michael,

Matlab's implementation of subplot always aligns the plot boxes. The "align" option rigidly aligns the axes "position" properties. Meaning as the axes labels and/or ticklabels grow in size the "position"s of the subplots do not change. This is also what Octave appears to be doing when "align" is specified.

In the sans "align" case, Matlab compensates the "position" property to ensure that the axes labels and ticklabels do not encroach beyond the "outerposition". Octave just treats each subplot as an independent axis with activepositionproperty = outerposition, and looseinset = [0 0 0 0].

I'm still have conceptual problems reconciling what I observe Octave to be doing and what is written in the code.

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Mon 08 Jul 2013 04:19:06 PM UTC, comment #5:

Ben,

OK. I found that now both rows of the subplot
require :
for k = 1:3
subplot (2, 3, K, "align"); % for first row
subplot (2, 3, K+3, "align"); % for second row
endfor
This produces correct alignment.

For some reason, after your previous fix, only
the second row required "align". In fact, I think
that the alignment went wrong if I used "align"
for the first row.

Anyhow, using "align" for both rows seems like the
appropriate usage. If that had worked before this
problem would not have come up.

However, there is currently no documentation for the "align"
option in the subplot section of the manual (or help).
And, there is no "noalign" for subplot. Since most
users of subplot would, I think, assume or prefer "align"
this should be the default. Then, it would make sense
to provide "noalign" to turn it off (which is what you
get now by default).

So, it looks like the only code change needed is the
setting of "align" by default, and provide the
"noalign" option. And, the documentation needs all this
described. I am not sure if the "noalign" option is
really needed, but someone might think of a useful case.
Since this is how it works now with "align" not set,
it is easy to allow.

Since it now works, this can have lower priority. But,
the documentation should be updated to match the current
code and include "noalign" if you think it is needed.

Michael Godfrey <godfrey>
Mon 08 Jul 2013 02:14:40 PM UTC, comment #4:

Ben,

I tried hg backout hg backout 16832:e528beb2d774
but that did not fix it. So, I started from an
up to date copy and did hg backout 16831:5893eb92361a

Neither of those had any effect. That is good in
a way, but...

I will see if I can find more, but you are the
expert in this code which, obviously, is complicated.

Michael Godfrey <godfrey>
Mon 08 Jul 2013 11:55:05 AM UTC, comment #3:

Michael,

My guess is that one of both of the changesets below tiggered the regression.

Initialization and updating of tightinset and looseinset properties.
http://hg.savannah.gnu.org/hgweb/octave/rev/e528beb2d774

Include "looseinset" in axes::properties::update_units ().
http://hg.savannah.gnu.org/hgweb/octave/rev/5893eb92361a

However, these actually fixed bugs, so I expect the problem is somewhere else. Iv'e been looking at graphics.cc (axes::properties::sync_positions). My c++ is rather poor, so the going is slow. However, my impression is that there is some code there that is extraneous (looseinset is set twice, the first time looks to be over-ridden by the second). Also the syncing is for the axes' outerposition, while Matlab syncs the axes' position. This is where the problem is (I think?).

For compatiblity,

  • subplot(n, m, k, "allign"); should treat the initial position property as fixed and allow the axes labels and ticklabels to encroach upon neighboring axes (implied activepositionproperty = position).
  • subplot(n, m, k); should treat the initial position property as flexible and allow the positions to shrink to prevent the axes labels and ticklabels from encroaching upon neighboring axes (implied activepositionproperty = outerposition).
  • In either case the subplot positions are aligned with the neighbors.

I'm skeptical I'll be able to fix this quickly. Especially since I'm not certain that the problem doesn't go beyond sync_positions. If you're upto looking into this, I could use the help.

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Mon 08 Jul 2013 12:35:04 AM UTC, comment #2:

Additional bit of information: It is a changeset
from before 3 July that broke this.

Michael Godfrey <godfrey>
Mon 08 Jul 2013 12:27:19 AM UTC, comment #1:

I looked more carefully. It looks more like the
plots in the first row have been made narrower.

Michael Godfrey <godfrey>
Mon 08 Jul 2013 12:22:37 AM UTC, original submission:

For a 3x2 subplot, the second row is sized
incorrectly. All three plots are too wide.
Trying uses of subplot(n, m, k, "allign");
changes the widths, but does not make them correct.

Ben, you pushed several patches for plotting yesterday
(july 6) can you guess which one did this? Or, should
I try backing each one out?

Michael Godfrey <godfrey>

 

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

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by bpabbott (Posted a comment)
  • -unavailable- added by rik5
  • -unavailable- added by godfrey (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 14 Aug 2013 03:42:40 PM UTCbpabbottStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Fri 19 Jul 2013 02:40:59 PM UTCbpabbottStatusNone=>Ready For Test
      Assigned toNone=>bpabbott
    Mon 08 Jul 2013 01:11:10 AM UTCrik5Carbon-Copy-=>Added bpabbott

    Back to the top


    Powered by Savane 3.1-cleanup