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

normalizeAxis now scales the input data with respect to the view area. #1034

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

maldag
Copy link

@maldag maldag commented May 4, 2018

No shift of data is computed.

This modified procedure significantly improves the outcome for 2d plots. It adds support to zoomed views of multiple datasets and limits the data to the spanwidth.

@egeerardyn egeerardyn self-requested a review May 4, 2018 13:32
@egeerardyn
Copy link
Member

From a quick look at the code, what I think has changed is:
instead of using XLim etc. to rescale the data, now the min/max of the present line plots is used.
Is that a correct summary?

I currently fail to see how this improves the situation, so could you elaborate and/or show an example (preferably with a picture or something illustrative).

My intuition tells me we should use XLim (etc.) and not the full data, since one may have a lot outside the active axis limits.

@maldag
Copy link
Author

maldag commented May 4, 2018

Using the YLim [-inf inf] for automatic scaling fails in the current solution. This was the originally indication for me to change this. It furthermore allows scaling to zoomed views, which the original mode does not.

@egeerardyn
Copy link
Member

Hmm, indeed, once one of the XYZLims is not finite, our current code will not do anything sensible; which should indeed be fixed (but I am a bit unsure whether this more complicated approach is the best way forward, especially since you mention quite a bit of it is still experimental).

@maldag
Copy link
Author

maldag commented May 4, 2018

I'm not well familiar with 3d plots. My implementation will fail trying to scale a 3d plot with automatic scaling. But the current implementation doesn't have a solution for this either.
The additional downside is the subtraction of the lower limit. This assumes the dataset is always positive. But a lot of technical plots are also negative. Thus, the assumption, the limits are [0,+inf] is wrong imho.

E.g. plotting deviations of a something or sines with mean not being zero. The mean must be kept, but scaled.

@egeerardyn
Copy link
Member

The additional downside is the subtraction of the lower limit. This assumes the dataset is always positive. But a lot of technical plots are also negative. Thus, the assumption, the limits are [0,+inf] is wrong imho.

I fail to see how negative data breaks by subtracting the lower limit (of the visible view). Could you show an example? By glancing at the code, for negative data, I still expect the visible part of the data to be mapped to the range [0, 1] in our code.

Basically, what our current code tries to do is come up with a translation and scaling of the data, such that the visible data falls in the range [0, 1], all the while without changing how the figure actually looks (but appropriately changing at what value the ticks are placed). The average value of the data is not pertinent to that.

@maldag
Copy link
Author

maldag commented May 4, 2018

In my testcase I used a plot with 5 Line Objects in R2017a.
The values on the Y Axis were in between [-2e-4;+3e-4] and included nans in the YData.
The former code scaled the data to [-2,+3], but not to [-1,+1] which I would expect or to [-0.5,+0.5]

If the original purpose of the code is the above, I misunderstood the meaning of it.

See example below for behaviour:

t=(0:0.001:0.2).';
y=2e-4*sin(2*pi*10*t)+0.8e-4*sin(2*pi*20*t);
y=y+0.1e-4*randn(length(y),1);

figure;
plot(t,y)


%%
hfig=figure;
plot(t,y)
cleanfigure('handle',hfig,'normalizeAxis','Y');

Your result: [-2,+3]
My result: [-1, +1]

@egeerardyn
Copy link
Member

Sorry for getting back after such a long time, I have not taken a lot of time lately to look at m2t since I don't use it that often anymore.

Anyway, I think there is a misunderstanding going on here: the goal of the axis normalization is to rescale the backing data in your axis to a more sensible range (i.e. the data stored in hLine.YData etc.). If you look at our current implementation, this is exactly what happens. However, the tick labels should remain unchanged (which they do not always do). So it is only the underlying data that gets scaled, not how the figure looks. The reason for doing tricks like this: the floating point precision in TeX is quite small (6 or 7 digits), so for plots that need a larger precision (e.g. a signal such as 1 + 1e-10*sin(t)), the rescaling will bil us out since we still work in MATLAB's double precision.

However, in the case you bring up, something is indeed not going the way it should: there is this little scale at the top of the Y axis which is not properly accounted for in our code (this is hAx.YRuler.Exponent), which causes the figure to indeed look different.

On one hand, the implementation that you propose is sub-optimal in a few aspects:

  • it only properly supports Line objects (since those are the only ones account for when you compute the scaling),
  • the code is a lot more complicated,
  • it does not support 3D plots (ours does),
  • it removes the tick handling (which it should not).
    For me, the last point makes it an absolute show stopper, as that breaks existing functionality. The other points also do not help.

So on our end, I see a few actions associated with this:

  • document the working premise of cleanfigure and normalizeAxis better (the figure should look identical!),
  • account for hAx._Ruler.Exponent in newer versions of MATLAB,
  • I am pretty sure that log axes will produce unwanted effects,
  • better handle everything when the axis limits are infinite (either bail out with a warning, or try to determine them in another way).

For that last point, I think indeed an approach like yours is a valid approach:

  • if the first limit is -Inf, compute the min over all _Data values of the axes' children (and ticks),
  • if the second limit +inf, computer the max over all _Data values of the axes' children (and ticks), but without the inherent coupling to Line objects.

But it needs to be done in a reliable way; while one can just as well set the limits by hand. So I'm not really convinced whether it is worth the additional work and complexity (since to handle zoomed plots, as you pint out, you somehow need to couple to the concrete Line/... classes since you do index into the data, and the code is implicitly assuming that Y depends on X (and that Y contains Inf in the limits), which is not how everybody's plots work).

Copy link
Member

@egeerardyn egeerardyn left a comment

Choose a reason for hiding this comment

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

See comment above. TL;DR:

  • breaks current functionality,
  • is really complicated to handle a small edge case that can be worked around easily.

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

2 participants