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

 
 

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

Submitted by:  Georg Wiora <gwiora>
Submitted on:  Mon 18 Sep 2017 09:14:13 AM UTC  
 
Category: PlottingSeverity: 3 - Normal
Priority: 5 - NormalItem Group: None
Status: FixedAssigned to: None
Originator Name: gwioraOpen/Closed: Closed
Release: 4.2.1Operating System: Microsoft Windows

Add a New Comment(Rich 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 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>
Project 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>
Project 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>
Project 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.

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.

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>
Project 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>
Project 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.

(file #41827)

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:

Pantxo Diribarne <pantxo>
Project 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:

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

BUT trying this stand alone

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:

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:

Replacing the last line by
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 File(s):
   
   
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
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by pantxo (Posted a comment)
  • -unavailable- added by gwiora (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 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 20 Sep 2017 04:56:51 AM UTCrik5StatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Tue 19 Sep 2017 12:11:57 PM UTCpantxoAttached File-=>Added validclim.patch, #41836
      StatusConfirmed=>Patch Submitted
    Mon 18 Sep 2017 08:13:57 PM UTCrik5StatusNeed Info=>Confirmed
      SummaryGUI becomes unresponsive when using slice() with logical array=>clim values should be cast to double before applying
    Mon 18 Sep 2017 02:52:20 PM UTCgwioraAttached File-=>Added MatlabTestResult.png, #41827
    Mon 18 Sep 2017 02:14:14 PM UTCpantxoStatusNone=>Need Info

    Back to the top


    Powered by Savane 3.1-cleanup1