bugGNU Octave - Bugs: bug #52988, Variable editor: allow scalars to...

 
 

bug #52988: Variable editor: allow scalars to be shown directly

Submitter:  Philip Nienhuis <philipnienhuis>
Submitted:  Sat 27 Jan 2018 09:05:52 PM UTC
   
 
Category:  GUI Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Feature Request
Status:  Fixed Assigned to:  None
Originator Name:  Philip Nienhuis Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 11 Feb 2018 06:46:36 PM UTC, comment #26: 

There may be some cruft left over from changes associated with this bug.  There is no slot connected to the following signal:


linux@ ~/octave/octave/octave $ grep maybe_resize_columns * -sir
libgui/src/variable-editor-model.cc:  emit maybe_resize_columns_signal ();
libgui/src/variable-editor-model.h:  void maybe_resize_columns_signal (void);


I.e., the following doesn't do anything:


linux@ ~/octave/octave/octave $ hg diff
diff --git a/libgui/src/variable-editor-model.cc b/libgui/src/variable-editor-mo
--- a/libgui/src/variable-editor-model.cc
+++ b/libgui/src/variable-editor-model.cc
@@ -1206,8 +1206,6 @@ variable_editor_model::update_data (cons

   emit dataChanged (QAbstractTableModel::index (0, 0),
                     QAbstractTableModel::index (new_display_rows-1, new_display
-
-  emit maybe_resize_columns_signal ();
 }

 void
diff --git a/libgui/src/variable-editor-model.h b/libgui/src/variable-editor-mod
--- a/libgui/src/variable-editor-model.h
+++ b/libgui/src/variable-editor-model.h
@@ -261,8 +261,6 @@ signals: // private

   void user_error_signal (const QString& title, const QString& msg) const;

-  void maybe_resize_columns_signal (void);
-
   void set_editable_signal (bool);

   void description_changed (const QString& description);


Was there something added and then removed inadvertently?

Dan Sebald <sebald>
Mon 29 Jan 2018 10:23:23 PM UTC, comment #25: 

Yes, this fixes the issue here.  Marking as fixed and closing report.

Unfortunately, it also revealed another problem that logical variables are automatically coerced into double values when editing.  I'll file a different report about that.

Rik <rik5>
Group administrator
Mon 29 Jan 2018 09:53:02 PM UTC, comment #24: 

I pushed the following changeset to fix the problem originally reported here:

http://hg.savannah.gnu.org/hgweb/octave/rev/ef60416c4686

I'm marking this report as Ready For Test.  If someone can confirm, can we please close this report and open separate reports for any additional problems?  I think the comments here have gone a bit off the rails.

John W. Eaton <jwe>
Group administrator
Mon 29 Jan 2018 09:36:30 PM UTC, comment #23: 

@jwe: Yes, the new patch fixes the resizing behavior of the columns.  This is turning in to one of those bug reports with a thousand different threads.  I am going to hive off the segfault, the performance issue with large arrays, and the display/editing of strings (i.e., character row vectors) in to separate bug reports.

This bug report can return to its original issue of displaying scalars directly rather than '[1x1 double]' when a cell array is present.

Rik <rik5>
Group administrator
Mon 29 Jan 2018 08:27:36 PM UTC, comment #22: 

RE: comment #19.  I don't know if you are saying there is a problem or what you are referring to in the comments of patch #8656.  If there is a problem now that is not about this bug report, then please open a new one and describe precisely how I can duplicate the problem.

John W. Eaton <jwe>
Group administrator
Mon 29 Jan 2018 08:20:29 PM UTC, comment #21: 

Does this patch fix the resize problem?

http://hg.savannah.gnu.org/hgweb/octave/rev/d88323efe969

John W. Eaton <jwe>
Group administrator
Mon 29 Jan 2018 06:49:02 PM UTC, comment #20: 

Updating my comment #18, the row height does work the way I expect it to.  I can set the row height to some particular value, and then update pieces of the matrix at will and the row height is preserved.

Rik <rik5>
Group administrator
Mon 29 Jan 2018 06:44:03 PM UTC, comment #19: 

Previously, while stepping through code, the variable editor pane contents always got updated even when the variable shown wasn't affected in the stepped command.  IIRC I made a note of it in patch #8656.  I noticed because I used the variable editor heavily while debugging (it is extremely helpful there); but because during debugging sessions speed usually is less critical I didn't bother much.

I don't know if this is related and if unneeded updating is still happening,  and I can't try now as I need to rush off and have no more time until tomorrow, sorry.

Philip Nienhuis <philipnienhuis>
Group Member
Mon 29 Jan 2018 06:38:37 PM UTC, comment #18: 

@jwe: I don't know if this counts as resizing, but here's what I get when Autofit is disabled.


x = magic (3);
openvar x
# Now resize the columns to be different than their default of 100
x(2,1) = pi;
# All columns resize to default of 100


I would have expected that no resizing occurs since it would be updating data values within the widget, but not recreating the widget from scratch.  If this is the way things are supposed to work, then could column resizing be disabled when Autofit is also disabled?  That would prevent people from making the mistake I did in thinking that their column size overrides would be maintained.


Rik <rik5>
Group administrator
Mon 29 Jan 2018 06:12:42 PM UTC, comment #17: 

I pushed this change:

http://hg.savannah.gnu.org/hgweb/octave/rev/456b486ae5e6

It should at least prevent resizing from happening when the autofit option is set to disabled.

John W. Eaton <jwe>
Group administrator
Mon 29 Jan 2018 06:06:34 PM UTC, comment #16: 

There is definitely something fishy with respect to synchronization and large variables in the Variable Editor.  After I have opened a large variable in the Editor, there is always a lag after any command in the command window, even simple ones.

Sample code:


x = rand (1000);
openvar x
# Delay in Command Window accepting keyboard input
y = 1
# another delay which prevents entering the next command immediately
y + 1
# another delay which prevents entering the next command immediately
z = y + 1



Rik <rik5>
Group administrator
Mon 29 Jan 2018 05:27:30 PM UTC, comment #15: 

Hmmm, that (comment #14) might relate to what I described in passing in comment #6 (see the BTW).
There it is the inverse -  setting Autofit apparently doesn't work (fully).

Philip Nienhuis <philipnienhuis>
Group Member
Mon 29 Jan 2018 05:24:29 PM UTC, comment #14: 

@Rik
I tried your example in comment #12 and I see the same. But -
I think it is a sort of delayed reaction by openvar - if I later on undock the variable editor (with x shown) and then resize it, there is no delay and the pane is updated promptly.

Trying again and double clicking rather than using openvar I see no delays either.
After a bit of experimenting I conclude it is definitely calling openvar as a command in the terminal that provokes the delay. Just clicking the border of the variable editor pane after entering openvar('x') makes Octave unresponsive for a while.

(Go figure - AFAIK openvar is called as well after double-clicking a variable in the workspace.)

(Windows 7 64b)

Philip Nienhuis <philipnienhuis>
Group Member
Mon 29 Jan 2018 05:24:28 PM UTC, comment #13: 

Is Autofit ever truly turned off?  I checked the preferences and the checkbox is unchecked.  However, when I do this


x = magic (3);
openvar x
# Columns are sized to a particular value large enough to hold an integer
x(2,1) = pi;
# First column resizes to hold floating point value.
# I didn't expect that



Rik <rik5>
Group administrator
Mon 29 Jan 2018 05:10:18 PM UTC, comment #12: 

I can also verify the painfully slow display of large matrices.  I used 'x = rand (1000)' to avoid the segfault I get with with 5000.

I turned off Autofit as well as "Plus font height".  After opening the variable in the Variable Editor I used the mouse to resize the widget and change the number of columns displayed.  This seemed to trigger a massive update event which made the GUI unresponsive.

Rik <rik5>
Group administrator
Mon 29 Jan 2018 04:58:06 PM UTC, comment #11: 

Should I break off the segfault question in to a separate bug report?  It is absolutely repeatable for me.  Here is the backtrace


#0  0x00007fcc6fb04428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007fcc6fb0602a in __GI_abort () at abort.c:89
#2  0x00007fcc7013e84d in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007fcc7013c6b6 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007fcc7013c701 in std::terminate() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007fcc7013c969 in __cxa_rethrow () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x00007fcc72edd92c in QVector<variable_editor_model::impl::cell>::reallocData (this=0x1aaa0b0, asize=25000000, aalloc=25000000, options=...)
    at /usr/include/x86_64-linux-gnu/qt5/QtCore/qvector.h:572
#7  0x00007fcc72edcf7b in QVector<variable_editor_model::impl::cell>::resize (this=0x1aaa0b0, asize=25000000) at /usr/include/x86_64-linux-gnu/qt5/QtCore/qvector.h:412
#8  0x00007fcc72edca42 in variable_editor_model::impl::reset (this=0x1aaa080, val=...) at libgui/src/variable-editor-model.cc:246
#9  0x00007fcc72ed8fa3 in variable_editor_model::variable_editor_model (this=0x1aa9300, expr=..., val=..., label=0x1a97440, parent=0x1a96090)
    at libgui/src/variable-editor-model.cc:333
#10 0x00007fcc72ecd412 in variable_editor::edit_variable (this=0x14e19a0, name=..., val=...) at libgui/src/variable-editor.cc:169
#11 0x00007fcc72e4f4ec in main_window::edit_variable (this=0x7fffa8e1c360, expr=..., val=...) at libgui/src/main-window.cc:1741
#12 0x00007fcc72ee9df6 in main_window::qt_static_metacall (_o=0x7fffa8e1c360, _c=QMetaObject::InvokeMetaMethod, _id=103, _a=0x7fcc303d8f90) at libgui/src/moc-main-window.cc:748
#13 0x00007fcc6ec2c719 in QObject::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#14 0x00007fcc6f57a04b in QWidget::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#15 0x00007fcc6f6924bb in QMainWindow::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#16 0x00007fcc6f534afc in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#17 0x00007fcc6f53a036 in QApplication::notify(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#18 0x00007fcc6ebff2a8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#19 0x00007fcc6ec0116a in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#20 0x00007fcc6ec53f53 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#21 0x00007fcc68517197 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#22 0x00007fcc685173f0 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#23 0x00007fcc6851749c in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#24 0x00007fcc6ec5435f in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#25 0x00007fcc6ebfcffa in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#26 0x00007fcc6ec056bc in QCoreApplication::exec() () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#27 0x00007fcc72e63cc7 in octave::gui_application::execute (this=0x7fffa8e1c6d0) at libgui/src/octave-gui.cc:248
#28 0x000000000040188d in main (argc=9, argv=0x7fffa8e1c9e8) at src/main-gui.cc:104


It looks like the realloc fails and so an exception is thrown, and then because of the intricacies of threads and signal handling, the signal handler we have installed is not catching it so it goes to the default std::terminate.

Rik <rik5>
Group administrator
Mon 29 Jan 2018 03:58:09 PM UTC, comment #10: 

Matlab (r2018a prerelease) shows the array immediately, just like Octave w/o Autofit set.
I didn't find a setting like "Autofit", just one called "Shrink tabs to fit" but that doesn't do anything AFAICS.

Philip Nienhuis <philipnienhuis>
Group Member
Mon 29 Jan 2018 02:07:02 PM UTC, comment #9: 

I'm not seeing segfaults, but I do see the performance issue with automatically adjusting the column widths for large arrays.

A 5000x5000 array requires almost 200MB for the numbers alone and more than that to cache the (short, limited to about 8 characters but sometimes more) string representations of the numbers and the associated QVector object that is used to store the info in the Qt model object.  With autofit enabled, I think Qt must generate the full list in the model.  With it disabled, then only what is displayed needs to be generated.  I suppose we could disable it by default for arrays larger than some limit.

Just out of curiosity, how does Matlab perform for a similar sized array?

John W. Eaton <jwe>
Group administrator
Mon 29 Jan 2018 03:28:00 AM UTC, comment #8: 

The behavior is quite a bit worse for me: segfault.


./run-octave -f
x = rand (5000);
openvar x
octave exited with signal 6



Rik <rik5>
Group administrator
Sun 28 Jan 2018 04:32:57 PM UTC, comment #7: 

@comment #5:
As long as strings and sq_strings can be edited I'm fine :-)
I don't think char arrays are used that much.

In short, it wouldn't hurt to follow Matlab here.

As to comments #4 and #6, I do note that performance improved greatly, nice!

Philip Nienhuis <philipnienhuis>
Group Member
Sun 28 Jan 2018 04:30:05 PM UTC, comment #6: 

@comment #4:
Just do:
- set "Autofit by column" in the Preferences | Variable Editor pane
- type: a = rand (5000);
- doubleclick a in the workspace.
On my core i5 Win7 PC it took Octave 2 minutes to show a in the variable editor.
BTW even then some columns still showed dots, double clicking between the column headers adapted the column width and then showed the full value.

Compare with:
- clear a
- unset "Autofit by column"
- type: b = rand (5000);
- doubleclick b in the workspace
On my PC, b showed up immediately in the variable editor.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 28 Jan 2018 04:17:03 PM UTC, comment #5: 

I'm not sure what to do with character arrays.  Matlab can't edit them.  We can, but I guess it should work differently from other numeric arrays.  So we would really be editing rows and simply taking what the user enters as the text instead of evaluating the expression to create a value.

John W. Eaton <jwe>
Group administrator
Sun 28 Jan 2018 04:13:59 PM UTC, comment #4: 

I think the performance has been greatly improved.  Previously, each time an element was requested from the model, an expression was evaluated in the interpreter.  Now we just maintain a copy of the value that is being edited (copy == increment a reference count) and when an element is needed, we just call the new edit_display function to create a string representation of the element for display in the editor.  So that is much faster than what was done before.  Does the autofit feature cause trouble for you now?  We can still make it configurable.  I also think it might be good to have minimum and possibly maximum column widths.  But those are secondary issues to making editing actually work.

John W. Eaton <jwe>
Group administrator
Sun 28 Jan 2018 02:00:17 PM UTC, comment #3: 

Thanks.

I also noted Octave now doesn't allow editing of string and sq_string objects anymore.  These used to be interpreted by the variable editor as (editable) char arrays, each cell one character.
The latter was not ideal; yet being able to edit a string or sq_string directly is very useful.

As to structs and struct arrays, I have some pics of what Matlab does in patch #8656, see bottom of comment #37.

A little OT:
One thing that may need thinking over is keeping the "Autofit by column width" setting for the variable editor. I found that setting this may lead to ridiculous long waiting times for large arrays. Maybe something that could provoke dedicated bug reports.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 28 Jan 2018 02:32:47 AM UTC, comment #2: 


> Also, does Matlab allow editing of character arrays? I mean things created with single quotes: ['foo'; 'bar'], not string objects.


No, it does not. It shows the following text, which can't be edited:


val =


    'foo'
    'bar'


Alexander Wilms <alexanderw>
Sun 28 Jan 2018 01:57:48 AM UTC, comment #1: 

Thanks for the feedback.  I've mostly been focusing on making the transfer of data between the interpreter and the gui simpler and more reliable.  Since this work turned into mostly rewriting things, it's not surprising that I lost a few features.

I agree that if a cell is a scalar value, it should be possible to edit it directly instead of opening a separate tab just for that.  I'll try to fix that tomorrow.

After that, I was planning to make editing struct arrays work.

Also, does Matlab allow editing of character arrays?  I mean things created with single quotes: ['foo'; 'bar'], not string objects.

John W. Eaton <jwe>
Group administrator
Sat 27 Jan 2018 09:05:52 PM UTC, original submission:  

After recent changes to the variable editor, cell array contents are shown only as a class, not value - see attached vared_b.png

Consider this code:

a = num2cell (rand (3));
a{2, 3} = "text";
a(3, 2) = struct ("b", 5);
a{2, 2} = num2cell (rand (2));
a(2, 1) = rand (2);


... and then open a in the variable editor.

Before those changes, scalar values in cells were shown as such, see attached vared_a.png, cells (1, :) and (2, 3).
Admittedly that previous behavior was not ideal and needed to be adapted - see e.g., cell {2, 2} where the contents are shown preceded by "{  [1, 1] = ".   For that case (nested cell array) I'd prefer to see the class is it is now after the changes ("2x2 cell"), like in attached vared_b.png.
For numeric arrays the output style is debatable; before the changes Octave used to show as much values as fitted in the cell;  Matlab does the same BTW. I think I like Octave's new behavior better.
The other /scalar/ cell contents (scalar doubles, strings) used to be shown directly as values. That is very helpful behavior while debugging; it avoids the need to keep clicking tab headers to follow scalar variable value changes across the cell array.

In summary, IMO it makes the variable editor a lot more usable if scalar contents of cell arrays can be shown as values rather than class.

FWIW, I've also attached a screenshot of Matlab's variable editor after having run the same code (r2018a prerelease) (vared_c.png).

JWE attached as he's currently working on the variable editor

Philip Nienhuis <philipnienhuis>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #43092:  vared_a.PNG added by philipnienhuis (11KiB - image/png)
file #43093:  vared_b.PNG added by philipnienhuis (15KiB - image/png)
file #43094:  vared_c.PNG added by philipnienhuis (9KiB - 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 sebald (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by alexanderw (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by philipnienhuis (Submitted the item)
  • -email is unavailable- added by philipnienhuis
  •  

    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
    2018-01-29 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2018-01-29 jwe StatusConfirmed Ready For Test
    2018-01-29 rik5 StatusNone Confirmed
    2018-01-27 philipnienhuis Attached File- Added vared_a.PNG, #43092
        Attached File- Added vared_b.PNG, #43093
        Attached File- Added vared_c.PNG, #43094
        Carbon-Copy- Added jwe

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code