bugGNU Octave - Bugs: bug #52053, clim values should be cast to...

 
 

bug #52053: clim values should be cast to double before applying

Submitter:  Georg Wiora <gwiora>
Submitted:  Mon 18 Sep 2017 09:14:13 AM UTC
   
 
Category:  Plotting Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Assigned to:  None
Originator Name:  gwiora Open/Closed:  * Closed
Release:  * 4.2.1 Operating System:  * Microsoft Windows
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 20 Sep 2017 04:56:51 AM UTC, comment #11: 

I pushed the patch to stable (http://hg.savannah.gnu.org/hgweb/octave/rev/ae35d50d9756).  Because caxis was ancient it was written in an unclear style and did not have any BIST tests.  I rewrote the function on the development branch here (http://hg.savannah.gnu.org/hgweb/octave/rev/f9e0f7f1e267).

Marking as fixed and closing report.

Rik <rik5>
Group administrator
Tue 19 Sep 2017 12:39:16 PM UTC, comment #10: 

@Georg: yes, imshow already takes care of setting clim depending on the image class, I did not change that behavior. As for imagesc that is not an issue.

Pantxo Diribarne <pantxo>
Group Member
Tue 19 Sep 2017 12:27:52 PM UTC, comment #9: 

Be careful with the image and imagesc etc. functions. They have a different behavior depending on the data type of the image:
Images using uint8 are assumed to be in the data range of 0 to 255, while images in float or double format are assumed to be in the data range 0 to 1.0. Color scaling and thus clim behavior depends on this data types. I do not now whether image-functions distinguish this by them self or if clim property does this.

Please check your patch not to change this behavior.

Georg Wiora <gwiora>
Tue 19 Sep 2017 12:11:57 PM UTC, comment #8: 

@Rik:
I used

>> grep -rl "\"clim\", " ./scripts


to determine which function to inspect. This gave me a little longer list than yours. Anyway, for _pie_, _bar_, _scatter_, _contour_, surfl, legend, values passed for "clim" property are already guaranteed to  be numeric.

The only affected functions are imagesc, imshow, slice, caxis. For the latter, an error is issued if the requested clim vector is not numeric and increasing. See attached patch.



(file #41836)

Pantxo Diribarne <pantxo>
Group Member
Mon 18 Sep 2017 08:13:57 PM UTC, comment #7: 

I can't reproduce the GUI hang on either Linux, or on a Windows XP VM.  However the error message is definitely reproducible.


>> [x,y,z]=meshgrid([1:5],[1:5],[1:5]);
>> vox = true(5,5,5);
>> vox(3,3,3)=false;
>> slice(x,y,z,vox,2,3,3)
error: invalid value for array property "clim"
error: called from
    slice at line 146 column 5


Given the testing, it does seem like trying to set clim to a logical value should fail.  That part is okay.  Maybe we just need to cast the clim values to double in slice before applying them as Pantxo suggests?

I used grep to find similar instances of setting clim in the m-files included in Octave's distribution.


image/image.m:188:          set (hax, "clim", double ([mn, mx]));
image/imagesc.m:106:        set (hax, "clim", climits);
image/imshow.m:207:    set (get (htmp, "parent"), "clim", display_range);
plot/appearance/caxis.m:92:    set (ca, "clim", [ax(1), ax(2)]);
plot/draw/private/__contour__.m:329:      set (ca, "clim", [min(lev)-1, max(lev)+1], "layer", "top");
plot/draw/private/__contour__.m:331:      set (ca, "clim", [min(lev), max(lev)], "layer", "top");
plot/draw/private/__scatter__.m:259:      set (ax, "clim", clim);
plot/draw/private/__scatter__.m:262:      set (ax, "clim", [clim(1), max(c(:))]);
plot/draw/slice.m:146:    set (hax, "clim", [minv, maxv]);


Interestingly, image.m already casts the values into double.  That was probably intentional.

Besides slice(), there are a few other instances that need to be reviewed.


Rik <rik5>
Group administrator
Mon 18 Sep 2017 03:40:31 PM UTC, comment #6: 

Ok so Octave is mostly compatible with ML here (I am unsure about "inf" handling for clim).

The error is solved by passing "double ([minv, maxv])" in slice. The GUI freeze is another story which I can't address since I can't reproduce.

Pantxo Diribarne <pantxo>
Group Member
Mon 18 Sep 2017 02:52:20 PM UTC, comment #5: 

See Appended screen-shot for matlab test result.

Matlab does also not allow boolean values for clim, but is a bit more verbose on the possible values.

clim seems always to be double type.


Georg Wiora <gwiora>
Mon 18 Sep 2017 02:30:04 PM UTC, comment #4: 

I think it is not uncommon to display a logical array as an image. So this situation should be handled silently.

Therefore it should be possible for clim to work with the numeric representation of boolean values 0 and 1 independent of matlab compatibility.

I hope to provide matlab result soon.

Georg Wiora <gwiora>
Mon 18 Sep 2017 02:14:14 PM UTC, comment #3: 

I can confirm the error but not the GUI freeze on linux.

For the error the question now is should we allow an array property like 'clim' to be filled with bool values or should we alter high level function so that they only pass numeric values. I think we need someone to test the following in Matlab:


hax = axes ();
set (hax, 'clim', [false, true]);
%% If there is no error above
class (get (hax, 'clim'))


Pantxo Diribarne <pantxo>
Group Member
Mon 18 Sep 2017 12:15:36 PM UTC, comment #2: 

To narrow down the problem I found the location in
.../m/plot/draw/slice.m line 144, where the error message is issued:

    minv = min (v(:));
    maxv = max (v(:));
    set (hax, 'clim', [minv, maxv]);


Array v contains the volume data as logical array in this situation. The failing command is

  set(hax,'clim',[false,true])


BUT trying this stand alone

  hax = newplot();
  set(hax,'clim',[false,true])


does not lead to a hanging GUI...

Georg Wiora <gwiora>
Mon 18 Sep 2017 09:49:17 AM UTC, comment #1: 

Before blocking the GUI octave issues an error message:


error: invalid value for array property &quot;clim&quot;


Georg Wiora <gwiora>
Mon 18 Sep 2017 09:14:13 AM UTC, original submission:  

Description

When trying to slice a logical array the gui becomes unresponsive, at least using qt on Windows.

Example

This example reproduces the behaviour:

[x,y,z]=meshgrid([1:5],[1:5],[1:5]);
vox = true(5,5,5);
vox(3,3,3)=false;
slice(x,y,z,vox,2,3,3)


Replacing the last line by

slice(x,y,z,double(vox),2,3,3)

produces the desired behavior.

=Suggested minimal solution=
Change documentation to state that logical arrays are not suitable.

=Better solution=
Allow use of logical arrays in slice.

=Background=
I found this problem, when using logical arrays in some other 3D-graphics plotting functions too. Maybe it is a generic problem in some lower level graphics function not cooperating with logical arrays.

Georg Wiora <gwiora>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #41836:  validclim.patch added by pantxo (3KiB - text/x-patch)
file #41827:  MatlabTestResult.png added by gwiora (12KiB - image/png - Matlab test-result)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by pantxo (Posted a comment)
  • -email is unavailable- added by gwiora (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-09-20 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2017-09-19 pantxo Attached File- Added validclim.patch, #41836
        StatusConfirmed Patch Submitted
    2017-09-18 rik5 StatusNeed Info Confirmed
        SummaryGUI becomes unresponsive when using slice() with logical array clim values should be cast to double before applying
    2017-09-18 gwiora Attached File- Added MatlabTestResult.png, #41827
    2017-09-18 pantxo StatusNone Need Info

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code