bugGNU Octave - Bugs: bug #38085, image package: fails to build with...

 
 

bug #38085: image package: fails to build with --enable-64 (entropyfilt)

Submitted by:  Carnë Draug <carandraug>
Submitted on:  Mon 14 Jan 2013 06:31:05 PM UTC  
 
Category: Octave Forge PackageSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Build Failure
Status: ConfirmedAssigned to: None
Originator Name: Open/Closed: Open
Release: devOperating System: Any

Add a New Comment (Rich MarkupRich 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)

Tue 07 Jul 2015 12:43:42 PM UTC, comment #12:

> After some trials, it was possible to get the code to compile on Linux amd64 + gcc-4.9.2 changing those two lines to:
>
> hist (((int)((int)(vals (i)))) + ((int)add)) ++;
>
> hist ((((int)(int)(vals (i)))) + ((int)add)) /= (double)len;


More important than being able to compile the code, does it still return correct results? That function doesn't have any tests so it's hard for me to test it. I have confirmed that it is not Matlab compatible but that is by design (and will have to be fixed later).

You seem to care about this function so I can assume that you have test cases for this function. Can you please confirm that the function still behaves properly?

> As additional info, using bwlabeln needs 19 Gbytes of working mem RAM to be capable of processing 1,3 GigaVoxel grayscale (uint8) data. bwconncomp ++ labelmatrix needs "only" 5 Gbytes. Does it seem reasonable?


bwlabeln is known to use more memory as it uses union-find (it is meant to be faster than bwconncomp on specific types of images but I haven't come across of such type yet). It also returns a matrix of class double (required for Matlab compatibility) so the return value of such image will be 10.4Gbytes. The way we work around border elements required a duplicate of the image. There are other approaches but they'll be slower. I would appreciate if you could implement alternatives.

If bwconncomp is faster and more memory efficient for you, use it.

Carnë Draug <carandraug>
Project Member
Mon 06 Jul 2015 07:42:21 PM UTC, comment #11:

Good evening to everyone,

On May we were trying to get a "linux 64bit integer build" to work with image package.

We were facing problems during the last 2 years with those kind of octave builds. It was pointed out by Jordi G.H. to make some changes in the spatial.cc code.

My mistake; at the moment of calling "make dist" to create the image.*.tar.gz it was not picking the spatial.cc that was modified. It is the first octave-forge package that we have edited and I was not capable of seeing that behaviour.

Anyway, I retried as Jordi explained, but it was also failing
the same way, saying again about "ambiguous conversion". I'm not too much into programming so I tried to force to convert those entities into standard ANSI C "int" variable.

After some trials, it was possible to get the code to compile on Linux amd64 + gcc-4.9.2 changing those two lines to:

hist (((int)((int)(vals (i)))) + ((int)add)) ++;

hist ((((int)(int)(vals (i)))) + ((int)add)) /= (double)len;

Those changes seem acceptable?
We have tried with bwlabeln, bwconncomp, labelmatrix and others, and the system has not failed.
As additional info, using bwlabeln needs 19 Gbytes of working mem RAM to be capable of processing 1,3 GigaVoxel grayscale (uint8) data. bwconncomp ++ labelmatrix needs "only" 5 Gbytes. Does it seem reasonable?

If You feel like there are other tests or changes that should be done, please let me know.

Thanks, yours sincerely Jose

José García <chechugarriga>
Tue 12 May 2015 06:43:36 PM UTC, comment #10:

> By the way, if we comment two of the three alternatives on the include file .h from octave,would it be possible to test/ debug that entropy function is working as expected ?


I'm not even sure that entropyfilt works correctly on 32-bit machine. The code on 2.4.0, in a 32 bit build, sometimes segfaults when image is of signed integer (already fixed). I have also managed to get into an endless loop sometimes.

This function has no tests to begin with. A quick comparison against Matlab already shows me the results differ but I think a big part of this is how it does the padding. There may be other problems with the function but before this bug report, I have never looked at the function.

If you are only interested on getting the same results as in a 32 bit build, just try it with a test image yourself and check if the results are similar.

Carnë Draug <carandraug>
Project Member
Tue 12 May 2015 05:52:32 PM UTC, comment #9:

Good evening everyone.
I tried with the change proposed by Jordi but the problem persists.
Tried on a fresh build of octave-4.0.0rc4.
It was said to change both lines to:

hist ((vals (i) + add).value ()) ++;

The problem is still present, gcc keeps informing of ambiguous alternatives.
By the way, if we comment two of the three alternatives on the include file .h from octave,would it be possible to test/ debug that entropy function is working as expected ?

Yours sincerely Chechu

José García <chechugarriga>
Tue 12 May 2015 05:44:30 PM UTC, comment #8:

According to a user on the mailing list, Jordi's proposed fix does not fix it:

http://octave.1599824.n4.nabble.com/Is-enable-64-still-experimental-tp4669274p4670354.html

Carnë Draug <carandraug>
Project Member
Mon 16 Mar 2015 03:41:36 PM UTC, comment #7:

The line numbers don't seem to match what I see in _spatial_.cc at revision faa0a913015f. Regardless, the problem seems to be with the following function:

A first weirdness is that it's templating the return value ET_OUT, yet it's always returning a double. I think the template argument ET_OUT is probably unnecessary, or else entropy should be changed to type ET_OUT so that it can use both float and double types.

As to the actual bug, I assume the problem is that `vals (i) + add` is returning octave_int8 values, but those can't be used for indexing. I don't even know why this used to work. I can't see what conversion path was used when octave_idx_type was int.

I think a simple fix might be to change those lines to

Jordi Gutiérrez Hermoso <jordigh>
Project Administrator
Sun 15 Mar 2015 04:15:51 AM UTC, comment #6:

Mike: That is what I thought. Just checking for the record.

Michael Godfrey <godfrey>
Sun 15 Mar 2015 03:10:44 AM UTC, comment #5:

Packages can be built and installed for both, but no a package built for 32-bit-indexed Octave won't work with 64-bit-indexed Octave, and vice versa.

Mike Miller <mtmiller>
Project Member
Sun 15 Mar 2015 02:36:30 AM UTC, comment #4:

Right, but can a package built with 64-bit indexing run
with 32-bit indexing Octave in any case?

Michael Godfrey <godfrey>
Sun 15 Mar 2015 02:28:31 AM UTC, comment #3:

No, this is not a release blocker. This relates to the image package anyway, not to any failure within Octave.

Mike Miller <mtmiller>
Project Member
Sun 15 Mar 2015 02:20:06 AM UTC, comment #2:

Mike: Just a clarification: is 64-bit integer build
supported for the 4.0 release? I thought that it is
not.

So if 64-bit is not not supported, this needs fixing,
but not before 4.0 release.

Michael Godfrey <godfrey>
Sun 15 Mar 2015 02:00:44 AM UTC, comment #1:

Confirmed, still fails to install using the current development branches of Octave (built with 64-bit index type) and the image package. At a glance the error messages look the same.

Mike Miller <mtmiller>
Project Member
Mon 14 Jan 2013 06:31:05 PM UTC, original submission:

The following bug was reported on the mailing list by Fredrik Lingvall <fredrik.lingvall@gmail.com>. Original can be seen on nabble.


I have Octave configured with --enable-64 and I can't get the image package to build with this setup:

Carnë Draug <carandraug>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -unavailable- added by chechugarriga (Posted a comment)
  • -unavailable- added by jordigh (Posted a comment)
  • -unavailable- added by godfrey (Posted a comment)
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by carandraug (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 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 12 May 2015 06:43:36 PM UTCcarandraugSummaryimage package: fails to build with --enable-64=>image package: fails to build with --enable-64 (entropyfilt)
    Sun 15 Mar 2015 02:16:42 AM UTCmtmillerDependencies-=>bugs #43908 is dependent
    Sun 15 Mar 2015 02:00:44 AM UTCmtmillerStatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup