bugGNU Octave - Bugs: bug #54261, blkmm error for some size of input


bug #54261: blkmm error for some size of input

Submitted by:  None
Submitted on:  Sat 07 Jul 2018 11:52:19 AM UTC  
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error
Status:  Fixed Assigned to:  None
Originator Name:  Originator Email:  -email is unavailable-
Open/Closed:  Closed Release:  4.4.0
Operating System:  GNU/Linux

Add a New Comment(Rich Markup)

You are not logged in

Please log in, so followups can be emailed to you.


Tue 10 Jul 2018 12:52:50 PM UTC, comment #5:

Thanks, that looks good to me.

John W. Eaton <jwe>
Project Administrator
Mon 09 Jul 2018 11:07:50 PM UTC, comment #4:

@jwe: I checked the patch in under your credentials here (http://hg.savannah.gnu.org/hgweb/octave/rev/e6b037514f56).

I amended it slightly. There seems to be a bit of a convention to use do_XXX for the internal function which implements the user visible XXX function. Hence, I renamed blkmm_ov to do_blkmm. I also rolled the templated blkmm and blkmm_ov in to the one function do_blkmm because they were both short and it didn't seem necessary to have so many templated functions. I added a BIST test for this bug as well.

Marking as fixed and closing report.

Rik <rik5>
Project Administrator
Mon 09 Jul 2018 10:11:15 PM UTC, comment #3:

I'm attaching a patch that I think eliminates as much duplicate code as possible. The blkmm template and sub-functions (but not the blkmm_ov template) could also be moved into liboctave.

(file #44530)

John W. Eaton <jwe>
Project Administrator
Mon 09 Jul 2018 08:09:18 PM UTC, comment #2:

@jwe: Maybe you can answer this question. The fix for this bug is conceptually simple, check for empty matrix inputs and return an empty matrix output if that is the case. However, in order to get the correct output type, double; double complex; single; single complex, it appears like the code to check for an empty matrix would need to be repeated four times. Simple, but ugly.

My proof of concept working only with double arrays is below

Is there any good way of avoiding the repetition of "if (m ==0 || n == 0)" within each branch of the if/else tree which decodes type?

If not, I'll just duplicate the tests.

Rik <rik5>
Project Administrator
Mon 09 Jul 2018 06:58:29 PM UTC, comment #1:

Confirmed. I suppose what is really needed is a test in blkmm for an empty matrix (m = 0 or p = 0). In that case, there is no need to call the underlying multiplication routines. Doing a little debugging with ordinary matrices

So in the following case, the output should be [](0x4x3), instead of an error.

Rik <rik5>
Project Administrator
Sat 07 Jul 2018 11:52:19 AM UTC, original submission:

Let size(A)=[m,p,q], size(B)=[p,n,q], C=blkmm(A,B), then size(C)=[m,n,q].

If (m=0 and n=1) or (m=1 and p=0 and n!=1), then blkmm will raise the following error:

This is a small issue, since it only raises errors for some size of empty input.

The following test code will show the above result.

and the output of the above code is as follow.



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

Attach Files:

Attached Files
file #44530:  diffs.txt added by jwe (6KiB - text/plain)


Depends on the following items: None found

Items that depend on this one: None found


Carbon-Copy List
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.




    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-07-09 rik5 StatusConfirmed => Fixed
        Open/ClosedOpen => Closed
    2018-07-09 jwe Attached File- => Added diffs.txt, #44530
    2018-07-09 rik5 Carbon-Copy- => Added jwe
    2018-07-09 rik5 StatusNone => Confirmed

    Back to the top

    Powered by Savane 3.4