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 Jan 14 18:31:05 2013  
 
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)

Thu Jun 2 07:36:47 2016, comment #13:

Good morning to everyone.

It's been a time since last additions to this bug.
We have been using image package for 3D images several months and the previously proposed changes seem to give correct results; althought we have not developed specific tests. (maybe it would be possible to launch commands with a common .mat file and compare it with the results given by image 2.5.1 and Matlab ones).
What do you think about it?
In addition, for future releases, it compiles without issue with gcc 5.3.1 and -std=gnu++11.

Cheers, chechugarriga.

José García <chechugarriga>
Tue Jul 7 12:43:42 2015, 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 Jul 6 19:42:21 2015, 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 May 12 18:43:36 2015, 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 May 12 17:52:32 2015, 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 May 12 17:44:30 2015, 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 Mar 16 15:41:36 2015, 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 Mar 15 04:15:51 2015, comment #6:

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

Michael Godfrey <godfrey>
Sun Mar 15 03:10:44 2015, 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 Administrator
Sun Mar 15 02:36:30 2015, 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 Mar 15 02:28:31 2015, 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 Administrator
Sun Mar 15 02:20:06 2015, 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 Mar 15 02:00:44 2015, 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 Administrator
Mon Jan 14 18:31:05 2013, 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 May 12 18:43:36 2015carandraugSummaryimage package: fails to build with --enable-64=>image package: fails to build with --enable-64 (entropyfilt)
    Sun Mar 15 02:16:42 2015mtmillerDependencies-=>bugs #43908 is dependent
    Sun Mar 15 02:00:44 2015mtmillerStatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup