bugGNU Octave - Bugs: bug #47687, Reassigned to another tracker...

 
 

bug #47687: Reassigned to another tracker [was: automatic broadcasting for assignment operations]

Submitter:  Carnë Draug <carandraug>
Submitted:  Wed 13 Apr 2016 12:01:37 PM UTC
   
 
Category:  Interpreter Severity:  1 - Wish
Priority:  5 - Normal Item Group:  Feature Request
Status:  None Assigned to:  None
Originator Name:  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

Sat 16 Apr 2016 07:25:11 PM UTC, comment #9: 

THIS ITEM WAS REASSIGNED TO PATCH #8983


Please, do not post any new comments to this item.

Carnë Draug <carandraug>
Group Member
Sat 16 Apr 2016 07:24:58 PM UTC, comment #8: 

@Lachlan

> [...] When you distinguish "local" from "the scope of the function", what is the difference? Does "local" mean just the current stack frame and "scope" mean the stack frame and all descendants?


Indeed, local usually means local to the function.  But some years ago, warning has an option named "local" which only affects the current stack frame and downstream.  You are well aware that warning states are global, but if you don't know about the "local" option", see the effect here:


function trigger_warn ()
  5 / 0;
endfunction

function foo ()
  disp ("trigger warning in subfunction");
  trigger_warn ();
  disp ("'locally' disable warning and cause warning in sub-function");
  warning ("off", "Octave:divide-by-zero", "local");
  trigger_warn ();
endfunction

warning ("query", "Octave:divide-by-zero")
foo ()
warning ("query", "Octave:divide-by-zero")


Which generates:


octave> warning ("query", "Octave:divide-by-zero")
ans =

  scalar structure containing the fields:

    identifier = Octave:divide-by-zero
    state = on

octave> foo ()
trigger warning in subfunction
warning: division by zero
warning: called from
    trigger_warn at line 2 column 5
    foo at line 3 column 3
'locally' disable warning and cause warning in sub-function
octave> warning ("query", "Octave:divide-by-zero")
ans =

  scalar structure containing the fields:

    identifier = Octave:divide-by-zero
    state = on


This reduced the use of unwind_protect blocks.  The use of word "local" for this option was unfortunate (there are several other like this, try 'lookfor ("-all", '"local"')').

So when I said 'scope of the caller only (different from the "local" option in warning)', I mean only the current stack frame.

> [...] suggesting "underline /", "underline *" etc for operations with broadcast. Any other combination of symbols would be fine. [...] Was that discussed at OctConf too?


No, only whether we should place the warnings back or remove the language extension warning.

> On an unrelated note, how would you feel about putting these "to-do" items in the task list, instead of the bug list?


I wrote a long text explaining that was not good because you can't move items between trackers.  Seems Like I was wrong.  The only thing I'll point out is that I feel less people subscribed to the task and patch tracker (based on the reduced number of replies I get when I report there).  I'll move this to the task tracker.

Carnë Draug <carandraug>
Group Member
Sat 16 Apr 2016 12:15:44 AM UTC, comment #7: 

lachlan: In comment 3 you asked about catching
braodcasts. I set something like:
warning ("error", "Octave:broadcast");
in my .octaverc depending on what I am
editing.

Michael Godfrey <godfrey>
Group Member
Fri 15 Apr 2016 12:45:31 AM UTC, comment #6: 

On an unrelated note, how would you feel about putting these "to-do" items in the task list, instead of the bug list?

The task list is quite underused, and triage is easier if wishlist items are separated from "it doesn't work" bugs.

Lachlan Andrew <lachlan>
Fri 15 Apr 2016 12:42:42 AM UTC, comment #5: 


> So unless Octave gets a new way to set warnings within the scope
> of the caller only (different from the "local" option in
> warning), such warning will be of quite limited use.


Developing a new warning system is on my "pipe-dream to-do list", mainly motivated by our previous discussion on this topic.  When you distinguish "local" from "the scope of the function", what is the difference?  Does "local" mean just the current stack frame and "scope" mean the stack frame and all descendants?


I'm not yet convinced that the usefulness of a broadcast warning would be all that limited.  Compare the number of core functions that use broadcasting with the number that use '#' for comments...

Anyway, I think a much better solution would be to replace "automatic" broadcasting with explicit broadcasting with a non-intrusive syntax.  The underlines in comment #3 were missed.  I as suggesting "underline /", "underline *" etc for operations with broadcast.  Any other combination of symbols would be fine.  (It is a pity that Octave uses ! and # to duplicate Matlab symbols, so there are fewer for new language features, but that's another discussion.)  Was that discussed at OctConf too?

Lachlan Andrew <lachlan>
Thu 14 Apr 2016 03:42:40 PM UTC, comment #4: 


> Why would this be nice?
>
> It is easy to use repmat for this situation.


Exactly because it avoids using repmat, and will perform faster.  This is exactly the same as numpy.

> As I have previously pointed out, automatic broadcasting masks many programming errors making debugging harder. Remember that the size of a may have been set in an entirely different function.


Isn't that the beauty of it?  You don't know the size of "a" so you don't have to worry about expanding "b" yourself.  And while you don't know the size of "a", you do know what it represents, and what each of its dimensions are.

> If you saw the line
>
> a(:,:) = b;
>
> in isolation, you would have no way of knowing whether or not broadcasting had been applied, or to what dimension. If you expected b to be a scalar, it would be nice for the system to warn you that it wasn't -- but we can't because we can't set warnings for automatic broadcast.


Actually, if "b" is a scalar this is automatic broadcasting.  It is expanding "b" singleton dimensions as necessary.

> Also notice that in all other cases, automatic broadcasting is symmetric in the dimensions of the two arguments. Would you expect
>
> a(1, 1:2) = eye (2);
>
> to give you a 1x2 or 2x2 result, or an error?


An error.  Assignment is not a symmetric operation, you can only expand the RHS.

> In our last discussion, you said that there should be no warning for automatic broadcasting because "it is part of the language".


It was not that straightforward but yes.  This then got further discussed in person during OctConf, including placing that warning back.  The argument is that automatic broadcasting is part of the language and it has an isue with Matlab incompatibility, therefore it goes in the same warning as all other language extensions.  But there is a real case to be made about having it as a separate error to prevent other errors (although maybe that only happened in the beginning, I haven't seen many error reports about it in a long while).

It has also been discussed elsewhere that the whole language-extensions warning is useless and I agree (we might as well just get rid of it. Octave is unusable with that warning enabled).  But that is an issue how warnings work in the language and it's required for Matlab compatibility.  Say we do make a separate warning for broadcast.  Normal use of Octave will be throwing warnings because octave core and packages do make use of broadcast on purpose.

So unless Octave gets a new way to set warnings within the scope of the caller only (different from the "local" option in warning), such warning will be of quite limited use.

Carnë Draug <carandraug>
Group Member
Thu 14 Apr 2016 04:22:30 AM UTC, comment #3: 

Michael, what default warning are you talking about?  AFAICT, there was a warning in 3.8.2 but that warning was removed in 4.0.0.

More specifically, the warning was merged into "Octave:language-extension", which is thrown so frequently by Octave's own scripts as to be unusable.

I have argued that it should be reinstated as a separate warning, but Carnë argued strongly against that, on the basis that it is "part of the language", as I mentioned below.

I entirely agree that broadcasting is useful, and it would be great to have new operators to do it more cleanly that bsxfun (akin to ./ .* etc, we could have / * etc or some such).  I just don't like the fact that it is compulsorily overloaded onto common operators.

Lachlan Andrew <lachlan>
Wed 13 Apr 2016 11:48:33 PM UTC, comment #2: 

Broadcasting is useful in cases where dimensions may
validly vary according to some requirements, such as
bus widths for example.

But, its accidental use can be dangerous and confusing.
Thus, the default warning is appropriate.
That is the way I use it.

Michael Godfrey <godfrey>
Group Member
Wed 13 Apr 2016 09:57:26 PM UTC, comment #1: 

Why would this be nice?

It is easy to use repmat for this situation.

As I have previously pointed out, automatic broadcasting masks many programming errors making debugging harder.  Remember that the size of  a  may have been set in an entirely different function.  If you saw the line


a(:,:) = b;


in isolation, you would have no way of knowing whether or not broadcasting had been applied, or to what dimension.  If you expected  b  to be a scalar, it would be nice for the system to warn you that it wasn't -- but we can't because we can't set warnings for automatic broadcast.

Also notice that in all other cases, automatic broadcasting is symmetric in the dimensions of the two arguments.  Would you expect


a(1, 1:2) = eye (2);


to give you a 1x2 or 2x2 result, or an error?

In our last discussion, you said that there should be no warning for automatic broadcasting because "it is part of the language".  If it is spreading further and further, then I'm tempted to compare it to a cancerous part of the language.

Unless there is a clear instance when this is useful, I will argue strongly against it.

Lachlan Andrew <lachlan>
Wed 13 Apr 2016 12:01:37 PM UTC, original submission:  

THIS ITEM WAS REASSIGNED TO PATCH #8983
Just like we support assignment to arrays with a scalar:


a = zeros (3, 4);
a(:,:) = 5;


It would be nice we could also broadcast for assignment, like so:


a = zeros (3, 4);
a(:,:) = [0:3];


Carnë Draug <carandraug>
Group Member

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by godfrey (Posted a comment)
  • -email is unavailable- added by lachlan (Posted a comment)
  • -email is unavailable- added by carandraug (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.

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-04-16 carandraug Reassign ItemGNU Octave, bug #47687 GNU Octave, patch #8983

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code