bugGNU Octave - Bugs: bug #43505, build gm with quantum-depth=16 in...

 
 

bug #43505: build gm with quantum-depth=16 in mxe octave

Submitted by:  Markus Bergholz <markuman>
Submitted on:  Thu 30 Oct 2014 04:50:58 PM UTC  
 
Category: Configuration and Build SystemSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Other
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: otherOperating 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 25 May 2016 07:55:42 PM UTC, comment #63:

works on 64 as well so pushed for mxe-octave:

http://hg.octave.org/mxe-octave/rev/5e57f06cd77f

John Donoghue <lostbard>
Project Member
Wed 25 May 2016 07:08:11 PM UTC, comment #62:

running on the 4.0.2 official gui, it crashes doing the clear all, which I believe was documented as on issue elsewhere.

running on the official 4.0.2 no gui,

John Donoghue <lostbard>
Project Member
Wed 25 May 2016 06:54:42 PM UTC, comment #61:

I saw that back on comment #39, but then could not reproduce it.

The clear all clears out private functions, and then doesn't know how to reload them ?

John Donoghue <lostbard>
Project Member
Wed 25 May 2016 06:32:33 PM UTC, comment #60:

Could you please also check this:

Avinoam Kalma <avinoam>
Project Member
Wed 25 May 2016 06:15:55 PM UTC, comment #59:

I am currently compiling a w64 version just to verify it runs and then will push the patch up

John Donoghue <lostbard>
Project Member
Wed 25 May 2016 06:13:59 PM UTC, comment #58:

The patch https://savannah.gnu.org/bugs/download.php?file_id=32344 should be applied to mxe-octave

Avinoam Kalma <avinoam>
Project Member
Wed 25 May 2016 03:52:53 PM UTC, comment #57:

Since John's tests was successful I'm marking this as fixed and closing.

Rik <rik5>
Project Administrator
Wed 25 May 2016 12:18:40 PM UTC, comment #56:

Rebuild worked:

John Donoghue <lostbard>
Project Member
Tue 24 May 2016 09:03:44 PM UTC, comment #55:

tip of stable branch

There might have been crud around, so will do a new rebuild and see if it works then

John Donoghue <lostbard>
Project Member
Tue 24 May 2016 08:56:13 PM UTC, comment #54:

Stable meaning released 4.0.2 version, or tip of the stable branch? After 4.0.2 it was moved into liboctinterp so there shouldn't be an oct-file with that name anymore.

Mike Miller <mtmiller>
Project Administrator
Tue 24 May 2016 08:44:31 PM UTC, comment #53:

And

>> _magick_read_

error: library open failed: C:\Octave\Octave-4.0.2\lib\octave\4.0.2\oct\x86_64-w64-mingw32\__magick_read__.oct

John Donoghue <lostbard>
Project Member
Tue 24 May 2016 08:35:22 PM UTC, comment #52:

On mine with a stable build:

Maybe my build wasn't clean

John Donoghue <lostbard>
Project Member
Tue 24 May 2016 04:58:21 AM UTC, comment #51:

As in comment #48, imwrite and imread passes all the tests. Tested with patch from bug #47968, on 32 bit Octave on Win-7, with qm=16.

Avinoam Kalma <avinoam>
Project Member
Mon 23 May 2016 08:05:07 PM UTC, comment #50:

I hadn't tried 64 bit, but will try a new build and verify

John Donoghue <lostbard>
Project Member
Mon 23 May 2016 05:57:34 PM UTC, comment #49:

So this can be committed now? AFAICT the failing tests were the only thing holding this up. The stable branch has the fix for bug #47746, so please test that everything works there (both 32-bit and 64-bit would be nice, since I had different behavior on Debian 32-bit vs 64-bit).

Mike Miller <mtmiller>
Project Administrator
Sun 22 May 2016 06:55:14 PM UTC, comment #48:

passed 20/20 for imwrite with the patch on bug #47746 applied

John Donoghue <lostbard>
Project Member
Thu 19 May 2016 04:32:50 PM UTC, comment #47:

I will take a look

John Donoghue <lostbard>
Project Member
Thu 19 May 2016 03:58:56 PM UTC, comment #46:

I've also submitted a patch on bug #47746, you can test it and see if it changes/fixes anything in the Windows build.

Mike Miller <mtmiller>
Project Administrator
Thu 19 May 2016 03:45:20 PM UTC, comment #45:

I don't know if this will make a difference, but I cleaned up imread.m and it's private implementation in _imread_.m. See this cset http://hg.savannah.gnu.org/hgweb/octave/rev/c86cacc3aaf4.

One of the things I re-coded was the parsing of the Index option.

Rik <rik5>
Project Administrator
Wed 18 May 2016 09:04:53 PM UTC, comment #44:

For the test that fails, its not the magick_read.cc:1113, but magick_read.cc:1147.

John Donoghue <lostbard>
Project Member
Wed 18 May 2016 06:48:05 PM UTC, comment #43:

and I just noticed that the 'gray' prints don't occur for the test that fails, which I guess is why I don't see any issues with the values that are printed.

John Donoghue <lostbard>
Project Member
Wed 18 May 2016 06:40:06 PM UTC, comment #42:

I will take back comment #40 - running the full _run_tests_ and the issue reappears.

However, looking at all of the printouts of gray, and none seem to have anything except lots of 0s after the decimal point.

John Donoghue <lostbard>
Project Member
Wed 18 May 2016 05:10:16 PM UTC, comment #41:

Ok, I don't know what is going on here, but it looks very different from bug #47746. The is_valid_index_option is a subfunction of _imread_.m, not sure how that can be undefined.

Mike Miller <mtmiller>
Project Administrator
Wed 18 May 2016 04:59:46 PM UTC, comment #40:

And if I comment out any calls to is_valid_index_option within _imread_.m, then test imwrite passes 20 out of 20 tests.

John Donoghue <lostbard>
Project Member
Tue 17 May 2016 09:00:07 PM UTC, comment #39:

If I manually run the test code that fails, when I get the imread in write_and_read_format, I get:

If I run the command without Index all - no error:

And if I run the test without the Index all options in it, that test passes,

however then on another test:

John Donoghue <lostbard>
Project Member
Tue 17 May 2016 08:49:34 PM UTC, comment #38:

precision added, but I don't see any decimals other than 0s.

When I run test imwrite it still fals though, but still what was printed out in the debug didn't show any decimals.

John Donoghue <lostbard>
Project Member
Tue 17 May 2016 03:10:44 PM UTC, comment #37:

Ok, you might need to add some more precision to this method of debugging, maybe "%.16lf". And show the values before and after calling a Java function that triggers the test failures in Windows.

The value I see on 32-bit Linux when this fails is something like 256.999999999998 rather than 257.

Mike Miller <mtmiller>
Project Administrator
Tue 17 May 2016 11:51:52 AM UTC, comment #36:

Added to _magick_read_.cc

When running imwrite (uint8(0:255), "test.tif") :

John Donoghue <lostbard>
Project Member
Mon 16 May 2016 06:37:21 AM UTC, comment #35:

I normally answer "y" to that question and it works for me once Octave has loaded its shared libraries.

Mike Miller <mtmiller>
Project Administrator
Mon 16 May 2016 01:16:48 AM UTC, comment #34:

I know am missing something ....

(gdb) break _magick_read_.cc:1113
No source file named _magick_read_.cc.
Make breakpoint pending on future shared library load? (y or [n])

John Donoghue <lostbard>
Project Member
Thu 12 May 2016 08:30:16 AM UTC, comment #33:

A patch will be good for me, but it will take me some time to build it...

Avinoam Kalma <avinoam>
Project Member
Thu 12 May 2016 12:38:09 AM UTC, comment #32:

Yeah, no rush. I may even have a patch for you to test by then.

If it's the same I'm seeing on Debian, then we want to look at this value each time through the loop:

This would be with any of the tests of imwrite that are failing, or with a simple example like

May need to also build with -O0, in my testing some important values were optimized out so I couldn't inspect them with -O2.

Mike Miller <mtmiller>
Project Administrator
Wed 11 May 2016 11:18:29 PM UTC, comment #31:

I can build a dev version this weekend if you are willing to wait that long

what do i need look for in gdb?

John Donoghue <lostbard>
Project Member
Wed 11 May 2016 10:22:50 PM UTC, comment #30:

Or I can provide a patch that you will have to apply to Octave for testing, in addition to the patch here of course to enable GraphicsMagick with 16-bit quantization, if building with a patch is easier for you than using a debugger.

Mike Miller <mtmiller>
Project Administrator
Wed 11 May 2016 09:44:24 PM UTC, comment #29:

Are there any Windows users or developers willing to do some testing of this with gdb? I believe this may be the same as bug #47746 on GNU/Linux. Moving to a 16-bit quantization introduces a potential for round-off error in the Octave routines when working with 8-bit images. If you have an mxe-octave build of the default branch with debug symbols not stripped and gdb available, I can tell you what to look for.

Mike Miller <mtmiller>
Project Administrator
Thu 26 Nov 2015 07:23:55 PM UTC, comment #28:

ok. After checking, the problem appears from imwrite.

After using java, the image saved by imwrite has all elements lower by one value. I have no idea where this may come from though.

Carnë Draug <carandraug>
Project Member
Wed 25 Nov 2015 09:51:16 PM UTC, comment #27:

last file

(file #35555)

Avinoam Kalma <avinoam>
Project Member
Wed 25 Nov 2015 09:50:00 PM UTC, comment #26:

Results:

files attached

(file #35551, file #35552, file #35553, file #35554)

Avinoam Kalma <avinoam>
Project Member
Wed 25 Nov 2015 09:38:39 PM UTC, comment #25:

I'm curious if the problem is on the reading or writing of the image. Could you run the following code:

And show the output and attach the tif and dat files generated?

By the way, I have fixed the warnings you are getting from randi on my own tree, and I'm just waiting from Kai to review it before pushing.

Carnë Draug <carandraug>
Project Member
Wed 25 Nov 2015 09:20:42 PM UTC, comment #24:

With official 4.0.0 version (compiled with quantum-depth=8)

Avinoam Kalma <avinoam>
Project Member
Wed 25 Nov 2015 09:16:51 PM UTC, comment #23:

Unfortunately, I have problems building last dev branch.
But I could build it two weeks ago, so here is the result:

Avinoam Kalma <avinoam>
Project Member
Wed 25 Nov 2015 04:31:51 PM UTC, comment #22:

Regarding comment #21, there have been odd interactions between Java and the rest of Octave before. In the end they have often indicated some place where Octave was sloppy about memory initialization or new/delete calls were unpaired. It's been a long time since I've run Octave under valgrind, but I bet there are still a few memory leaks.

It would be good for Avinoam to re-test a recent version of the dev branch to see if there is still a problem. I'm hoping that the extensive changes to the core code to use exceptions may have altered the problem behavior.

Rik <rik5>
Project Administrator
Wed 25 Nov 2015 01:06:38 PM UTC, comment #21:

@John Donoghue:

> So do we want quantum 16 ?


We definitely want quantum depth 16. Maybe not for 4.0.1 since that's a patch release but we will want it for 4.2. It should have been quantum depth 16 since the beginning.

@Avinoam:

Can you confirm the behaviour you saw again? About the image tests failing after the javaObject call? Is that always reproducible? I don't understand how these can be related. I specially don't understand how it can be related to quantum depth. Can you show:

With and without this patch?

Carnë Draug <carandraug>
Project Member
Wed 25 Nov 2015 12:23:39 PM UTC, comment #20:

We do want quantum 16, but we can't do it because imread/imwrite failure.

please see comment #17, the command

jobj = javaObject ("java.lang.StringBuffer");

is the reason for the failure. How can we fix this?

Avinoam Kalma <avinoam>
Project Member
Wed 25 Nov 2015 01:13:17 AM UTC, comment #19:

Currently mxe does not use quantum 16.

With quantum 16, the tests fail, without, they do not.

So do we want quantum 16 ?

John Donoghue <lostbard>
Project Member
Wed 25 Nov 2015 12:38:16 AM UTC, comment #18:

Doesn't the development branch now use quantum-depth=16 for MXE Octave? Can this report be closed?

Rik <rik5>
Project Administrator
Fri 01 May 2015 12:32:00 PM UTC, comment #17:

Revisiting this bug, I have compiled MXE_octave rc4 with quantum-depth=16, and check with run_test_suite.
Again, I got failures in imread and in imwrite.

If I check imread and imwrite before run_test_suite they pass.

I have found that:

test imread -> pass
test imwrite -> pass

jobj = javaObject ("java.lang.StringBuffer");

test imread -> fail
test imwrite -> fail

In run_test_suite the command with javaObject appears in test ov-class.cc-tst, before test imread & imwrite

Any explanation?

Avinoam Kalma <avinoam>
Project Member
Tue 02 Dec 2014 07:53:23 PM UTC, comment #16:

Hi,

strange results:

I have reproduced the bug that John Donoghue saw (comemnt #1)
in octave-cli version, but it did not appear in
octave-gui version.

Any explanation for such behaviour?

Avinoam

Avinoam Kalma <avinoam>
Project Member
Fri 21 Nov 2014 10:28:40 PM UTC, comment #15:

Hi,

I run the two versions, the one with quantum-depth=8 and that of
quantum-depth=16.

I got the same failure in both runs, but I didn't get the failures in imwrite that that John Donoghue experienced and Carnë Draug explained.

Avinoam

Avinoam Kalma <avinoam>
Project Member
Thu 20 Nov 2014 03:34:46 PM UTC, comment #14:

@Carnë Draug,

It seems that all the errors I got are not related
to quantum-depth=16.
I have sent them to answer John Donoghue's question.

I will run the quantum-depth=8 version and see if I get the same errors.

Thanks,

Avinoam

Avinoam Kalma <avinoam>
Project Member
Thu 20 Nov 2014 02:28:44 PM UTC, comment #13:

@Avinoam

I am unsure of the problem you are getting. Did you had more errors after building with quantum-depth=16? Or were those errors already there? My explanation was for the errors that John Donoghue experienced and which you do not.

Carnë Draug <carandraug>
Project Member
Wed 19 Nov 2014 09:15:39 PM UTC, comment #12:

The failures I got are:

note that 2nd run has 3 more failures (all in ctor-vs-method.tst ).

Avinoam

Avinoam Kalma <avinoam>
Project Member
Wed 19 Nov 2014 09:06:31 PM UTC, comment #11:

Carnë Draug, thanks for the detailed explanation.

What do you recommend to do now?

Avinoam

Avinoam Kalma <avinoam>
Project Member
Wed 19 Nov 2014 05:24:18 PM UTC, comment #10:

The failing test is real, I have seen it reported a few times (I don't remember if ever happened to me though). The reason is the following:

  • imwrite and imread are tested by creating random images. These are always uint8 because if the user does have GM installed, all builds will support this.
  • the whole reason why quantum depth is a build option is because then GM will ONLY handle images of that bitdepth. What this means is that when it reads an uint8 and is built with quantum depth 16, it will read them scaled in the 16bit range. Example, if the file has uint8 values but GM was built with quantum depth 16, then GM will give the value 65535. The same happens for writing, i.e., you must give a value of 65535 when you actually want to write a 255 uint8.
  • this means that rounding errors can creep but I don't know where (and I can't replicate). I'll guess that some autoconf macros would be required to modify how the rounding is done depending on the system where it is built.

Alternatives are: 1) live with the error, 2) ditch GM.

Carnë Draug <carandraug>
Project Member
Wed 19 Nov 2014 12:50:43 AM UTC, comment #9:

What are the errors that occur when running run_test_suite ?

John Donoghue <lostbard>
Project Member
Tue 18 Nov 2014 10:18:50 PM UTC, comment #8:

Hi again,

I have found "fntest.log", as you said (the "/test" should be removed).

after _run_test_suite_

strange. How can we debug this?

Avinoam

Avinoam Kalma <avinoam>
Project Member
Tue 18 Nov 2014 10:00:52 PM UTC, comment #7:

Sorry, I meant _run_test_suite_

fntests.log will be in the directory you started in when running _run_test_suite_.

After running the test suite, also then try running test imwrite

John Donoghue <lostbard>
Project Member
Tue 18 Nov 2014 09:45:17 PM UTC, comment #6:

Hi,

I did not find the run_tests script, so I run

There were 16 fails, (none of them was imwrite).
The version I have used is the mxe-octave + gm-mxe.patch.
BTW, I did not find the file test/fntests.log.

Avinoam

Avinoam Kalma <avinoam>
Project Member
Tue 18 Nov 2014 02:31:45 PM UTC, comment #5:

On my machine, it passes on running standalone:

However running it with _run_tests_ OR after running it after running _run_tests_, it fails:

John Donoghue <lostbard>
Project Member
Sun 16 Nov 2014 09:13:42 PM UTC, comment #4:

Thanks andy.

I run this command, using last Octave version from http://hg.octave.org/mxe-octave/ with gm-mxe.patch

@lostbard: can you check again? how can we debug this problem?

Thanks,

Avinoam

Avinoam Kalma <avinoam>
Project Member
Sat 15 Nov 2014 08:32:45 PM UTC, comment #3:

@Avinoam: In this case at the end of imwrite.m. The %!test blocks.

You can run the tests with "test imwrite" from within Octave.

Andreas Weber <andy1978>
Project Member
Sat 15 Nov 2014 06:36:05 PM UTC, comment #2:

Hi,

Where can I find imwrite tests?

Thanks,

Avinoam

Avinoam Kalma <avinoam>
Project Member
Mon 10 Nov 2014 10:03:48 PM UTC, comment #1:

On applying this patch,and running the test suite, I get failures when running imwrite tests:

John Donoghue <lostbard>
Project Member
Thu 30 Oct 2014 04:50:58 PM UTC, original submission:

Graphicsmagick is not build with quantum-depth=16 in mxe octave...patch attached

Markus Bergholz <markuman>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #35555:  data-before-read.mat added by avinoam (821B - application/octet-stream)
file #35551:  data-afterjava.tif added by avinoam (370B - image/tiff)
file #35552:  data-beforejava.tif added by avinoam (370B - image/tiff)
file #35553:  data-afterjava-read.mat added by avinoam (820B - application/octet-stream)
file #35554:  data-beforejava-written.dat added by avinoam (709B - application/octet-stream)
file #32344:  gm-mxe.patch added by markuman (808B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by andy1978 (Posted a comment)
  • -unavailable- added by avinoam (Posted a comment)
  • -unavailable- added by avinoam
  • -unavailable- added by lostbard (Posted a comment)
  • -unavailable- added by markuman (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 11 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 25 May 2016 03:52:53 PM UTCrik5StatusIn Progress=>Fixed
      Open/ClosedOpen=>Closed
    Wed 25 Nov 2015 09:51:16 PM UTCavinoamAttached File-=>Added data-before-read.mat, #35555
    Wed 25 Nov 2015 09:50:00 PM UTCavinoamAttached File-=>Added data-afterjava.tif, #35551
      Attached File-=>Added data-beforejava.tif, #35552
      Attached File-=>Added data-afterjava-read.mat, #35553
      Attached File-=>Added data-beforejava-written.dat, #35554
    Wed 25 Nov 2015 04:31:51 PM UTCrik5StatusNone=>In Progress
    Wed 25 Nov 2015 01:06:38 PM UTCcarandraugRelease3.8.2=>other
    Thu 13 Nov 2014 10:48:22 PM UTCavinoamCarbon-Copy-=>Added avinoam
    Thu 30 Oct 2014 04:50:58 PM UTCmarkumanAttached File-=>Added gm-mxe.patch, #32344

    Back to the top


    Powered by Savane 3.1-cleanup1