bugGNU Octave - Bugs: bug #53435, tests: camorbit.m, camroll.m: two...

 
 

bug #53435: tests: camorbit.m, camroll.m: two failing tests on i686

Submitted by:  Mike Miller <mtmiller>
Submitted on:  Fri 23 Mar 2018 06:34:33 AM UTC  
 
Category:  Test Suite Severity:  3 - Normal
Priority:  1 - Later Item Group:  Inaccurate Result
Status:  Postponed Assigned to:  None
Originator Name:  Open/Closed:  Open
Release:  dev 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.

 

( Jump to the original submission)

Mon 09 Apr 2018 01:39:54 AM UTC, comment #15:

@Markus: I checked in your cset for camroll here http://hg.savannah.gnu.org/hgweb/octave/rev/f83c2015cbc7.

Rik <rik5>
Project Administrator
Sun 08 Apr 2018 01:21:38 PM UTC, comment #14:

There was an error in camroll (and apparently in earlier Matlab versions) that the dataaspectratio was not considered. The attached patch should fix that error and allows the tests to pass with the updated reference values from recent Matlab versions.

It should be save to apply this patch on stable because no big changes are made (and it fixes a bug).

That still leaves the original issue why the tests need broader tolerances on 32bit architectures. But that definitely has time for after 4.4 is released.

(file #43840)

Markus Mützel <mmuetzel>
Project Member
Sun 08 Apr 2018 08:46:58 AM UTC, comment #13:

@Dmitri and Tik: That's what I tried to point out in comment #8. Something seems to have changed between Matlab versions that changed how camroll calculates the CameraUpVector. In the meantime, Himanshu Bhadani confirmed on the mailing list that he sees the same (new) results with Matlab R2017b. [1]

The changeset with the updated reference values isn't really needed for the 4.4 release. We could either revert in on stable (and only apply on default). Or we could mark the failing test as a known bug.
I didn't have time yet to check whether there is an easy fix. But I will try and check today.

[1]: http://octave.1599824.n4.nabble.com/Matlab-test-for-camup-td4687417.html

Markus Mützel <mmuetzel>
Project Member
Sat 07 Apr 2018 11:35:32 PM UTC, comment #12:

But error is just a way off...
I thought meya be there is a typo somewhere.

Dmitri.

Dmitri A. Sergatskov <dasergatskov>
Sat 07 Apr 2018 11:32:33 PM UTC, comment #11:

Maybe the patch with updated values also returned to tighter tolerances which had been loosened by Mike in order to get everything to pass?

Rik <rik5>
Project Administrator
Sat 07 Apr 2018 11:22:19 PM UTC, comment #10:

I just saw on stable buildbot:

http://buildbot.octave.org:8010/builders/stable-gcc-fedora/builds/6/steps/test/logs/stdio

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Sat 07 Apr 2018 10:42:34 PM UTC, comment #9:

I pushed the updated tolerances to the stable branche here (http://hg.savannah.gnu.org/hgweb/octave/rev/c578827b2eaf).

Rik <rik5>
Project Administrator
Tue 03 Apr 2018 04:53:06 PM UTC, comment #8:

The attached patch updates the references to the highest possible precision as seen in Matlab R2016a.

Most changes only add a few more decimals. But the test in camroll.m gives very different results from the previous ones (R2014a):

The on-screen results in R2016a seem to be reasonable. Maybe someone with a newer Matlab version might be able to check whether this is only an error in Matlab R2016a on Windows 10 64bit.

Code to execute:

(file #43783)

Markus Mützel <mmuetzel>
Project Member
Mon 26 Mar 2018 08:39:51 PM UTC, comment #7:

I pushed the test change on the stable branch

https://hg.savannah.gnu.org/hgweb/octave/rev/a95140167457

Leaving this bug report open for investigation of why these tolerance increases are necessary.

Mike Miller <mtmiller>
Project Administrator
Sat 24 Mar 2018 03:14:11 PM UTC, comment #6:

@Mike: I won't be able to check in Matlab probably for a week or two. But go ahead and push your patch.
I'll try and get results with more significant digits later. But as I understand our GL based graphics are single precision anyway (and the differences are very minor). Thus, relaxing the tolerance is totally fine.

I am guessing the underlying issue is that some trigonometric functions give slightly different results on different platforms. I believe there is a bug report about this somewhere. But I cannot find it right now.

Markus Mützel <mmuetzel>
Project Member
Fri 23 Mar 2018 09:57:38 PM UTC, comment #5:

Here is the patch that loosens the tolerances enough to pass for my testing. I will keep this pending for now and won't push this until I hear back, but this should be addressed for the 4.4 release.

(file #43654)

Mike Miller <mtmiller>
Project Administrator
Fri 23 Mar 2018 05:03:19 PM UTC, comment #4:

Markus, do you have access to Matlab to get updated constants for the tests? Besides these test failures, I see explicit floating point constants from Matlab in tests for camlookat.m, camorbit.m, camroll.m, camzoom.m, and camlight.m. Matlab doesn't have printf, so that would be sprintf.

If you think this will take a while to investigate or don't have time now, I will just increase the tolerances for now and leave this report open, as with bug #53438.

Mike Miller <mtmiller>
Project Administrator
Fri 23 Mar 2018 03:42:29 PM UTC, comment #3:

Note that IEEE-854 doubles have 15-17 significant digits when expressed in base 10. The constants have only 15 significant digits. The first step should be to update the precision of the constants used in comparison to be 17 digits. This is longer than what is typically displayed with "format long" so you need to use printf with a format. For example,

Rik <rik5>
Project Administrator
Fri 23 Mar 2018 07:29:13 AM UTC, comment #2:

I'd be happy to test further if you can tell me what to run.

Here's the output from running the minimal commands directly to show the camorbit results that are different:

Mike Miller <mtmiller>
Project Administrator
Fri 23 Mar 2018 07:07:05 AM UTC, comment #1:

Increasing the tolerances for these tests (on stable) should be fine.
It might make sense to track down which underlying operation is causing the deviation and whether that is a known problem.

Markus Mützel <mmuetzel>
Project Member
Fri 23 Mar 2018 06:34:33 AM UTC, original submission:

In testing Octave built for i686, I am seeing 1 test failure in camorbit.m and 1 test failure in camroll.m.

Both test failures look like candidates for increasing the tolerances, if someone familiar with these functions can verify that this makes sense.

Mike Miller <mtmiller>
Project Administrator

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #43840:  bug53435_camroll.patch added by mmuetzel (3KiB - application/octet-stream)
file #43783:  bug53435_cam_reference.patch added by mmuetzel (7KiB - application/octet-stream)
file #43654:  bug53435.diff added by mtmiller (1KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dasergatskov (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by mtmiller (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 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-04-08 mmuetzel Attached File- => Added bug53435_camroll.patch, #43840
    2018-04-03 mmuetzel Attached File- => Added bug53435_cam_reference.patch, #43783
    2018-03-26 mtmiller Priority5 - Normal => 1 - Later
        StatusPatch Submitted => Postponed
    2018-03-23 mtmiller Attached File- => Added bug53435.diff, #43654
        StatusNone => Patch Submitted

    Back to the top


    Powered by Savane 3.4