[patch] Getting rid of the lapack dependancy?

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch] Getting rid of the lapack dependancy?

Thomas Capricelli

Hi,

I was very surprised (shocked?) to read http://blog.akhuettel.de/2010/09/first-bump-digikam-140-kipi-plugins-130.html

Lot of people, and especially Gentoo people, think it's very bad to bundle copies of libraries (btw, I fully agree with them, but that's not the point here). Hence the gentoo package for digikam tries to use the systemwide lapack, which requires blas, and hence a fortran compiler.

Looking at the code, i could only see one single call to lapack : "dgesv_()" in libs/dimg/filters/sharp/matrix.cpp, which is a basic Ax=b solving (LU with pivoting). So one, and only one solving using a very common method makes digikam requires lapack/blas/fortran in gentoo, or, to stick with digikam point of view, lot of 3rdparty/ files in digikam source.

BUT, digikam already depends on KDE, which itself requires Eigen (http://eigen.tuxfamily.org), which provides such kind of linear algebra, and moreover in a very efficient manner, both code/API wise, and from a performance point of view.

Moreover, eigen was developed inside KDE to answer linear algebra needs among different KDE projects. Though it now lives its own way, there are still strong connections with KDE.

As you've probably guessed by now, I'm proposing a patch to do right this : use eigen instead of lapack to solve the linear system. Again, the dependancy on eigen is weak, as KDE already depends on it. I've used Eigen2 because eigen3 is not released yet (it's still in beta), and also because kde still depends on eigen2. I'll be happy to update those few lines to eigen3 when required. In case you wonder, eigen works with a very broad range of compilers, oses, platforms, cpus... so it should in no way constrain digikam.

I've read http://www.digikam.org/drupal/contrib and the "HACKING" file and think/hope i comply with those. The webpage has great information such as "send patches this way", but no indication of where those patches should be sent, I hope this is the right place. By the way, i have write access to kde svn, but I of course would not dare committing such a patch without asking you first. That can be useful later on though.

I've not tested this yet, mostly because i dont know how to test it, and because i first want to get your 'go' on this before spending more times on the patch. At least it compiles, and i think it does the same as before, there really are few lines to consider. The return value from the lapack function was not used (it could tell if the system has no solution), so i've kept it this way.

If you happen to agree on the idea, I could test it with someone with more in-depth knowledge of this code, and then commit it. I'd like to make some performance tests, too, just out of curiosity.

The source code in libs/dimg/filters/sharp/ is really weird by the way, it's supposed to be C++, but it's actually struct, with lot of c-like functions such as 'create matrix', 'free matrix', 'copy matrix', and such. It looks like gtk. My main purpose was to remove the lapack dependancy/code duplication, so i did a minmalist patch, but there's a lot of room for improvement here, by switching to actual C++, and maybe use slightly more of eigen (convolution and other stuff).

Oh, and, last thing, i'm not subscribed to [hidden email], please CC: me. I'll try to look at the list archive.

best regards,

--

Thomas Capricelli

http://www.freehackers.org/thomas


_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

digikam-move-to-eigen.diff.bz2 (61K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [patch] Getting rid of the lapack dependancy?

Gilles Caulier-4
Thanks for your patch...

The reason of clapack inclusion in digiKam core is due to use excelent
refocus algorithm taken from an old gimp plugin, rewritten and
improved a little bit to support 16 bits color depth images.

http://refocus.sourceforge.net/

As you can see there :

http://refocus.cvs.sourceforge.net/viewvc/refocus/refocus/lib/

refocus algorithm use an embeded version of clapack code as well. I
personally included this algorithm in digiKam core and of course, this
include clapack code.

Your patch is interresting. I recommend to create an entry in KDE
bugzilla and attach your patch as well. It always better to use
bugzilla instead the mailing list about patch, because we save the
story in time. Nothing is lost. I will review later, when time permit
your patch to include it in digiKam.

Thanks in advance

Gilles Caulier

2010/9/17  <[hidden email]>:

> Hi,
>
> I was very surprised (shocked?) to read
> http://blog.akhuettel.de/2010/09/first-bump-digikam-140-kipi-plugins-130.html
>
> Lot of people, and especially Gentoo people, think it's very bad to bundle
> copies of libraries (btw, I fully agree with them, but that's not the point
> here). Hence the gentoo package for digikam tries to use the systemwide
> lapack, which requires blas, and hence a fortran compiler.
>
> Looking at the code, i could only see one single call to lapack : "dgesv_()"
> in libs/dimg/filters/sharp/matrix.cpp, which is a basic Ax=b solving (LU
> with pivoting). So one, and only one solving using a very common method
> makes digikam requires lapack/blas/fortran in gentoo, or, to stick with
> digikam point of view, lot of 3rdparty/ files in digikam source.
>
> BUT, digikam already depends on KDE, which itself requires Eigen
> (http://eigen.tuxfamily.org), which provides such kind of linear algebra,
> and moreover in a very efficient manner, both code/API wise, and from a
> performance point of view.
>
> Moreover, eigen was developed inside KDE to answer linear algebra needs
> among different KDE projects. Though it now lives its own way, there are
> still strong connections with KDE.
>
> As you've probably guessed by now, I'm proposing a patch to do right this :
> use eigen instead of lapack to solve the linear system. Again, the
> dependancy on eigen is weak, as KDE already depends on it. I've used Eigen2
> because eigen3 is not released yet (it's still in beta), and also because
> kde still depends on eigen2. I'll be happy to update those few lines to
> eigen3 when required. In case you wonder, eigen works with a very broad
> range of compilers, oses, platforms, cpus... so it should in no way
> constrain digikam.
>
> I've read http://www.digikam.org/drupal/contrib and the "HACKING" file and
> think/hope i comply with those. The webpage has great information such as
> "send patches this way", but no indication of where those patches should be
> sent, I hope this is the right place. By the way, i have write access to kde
> svn, but I of course would not dare committing such a patch without asking
> you first. That can be useful later on though.
>
> I've not tested this yet, mostly because i dont know how to test it, and
> because i first want to get your 'go' on this before spending more times on
> the patch. At least it compiles, and i think it does the same as before,
> there really are few lines to consider. The return value from the lapack
> function was not used (it could tell if the system has no solution), so i've
> kept it this way.
>
> If you happen to agree on the idea, I could test it with someone with more
> in-depth knowledge of this code, and then commit it. I'd like to make some
> performance tests, too, just out of curiosity.
>
> The source code in libs/dimg/filters/sharp/ is really weird by the way, it's
> supposed to be C++, but it's actually struct, with lot of c-like functions
> such as 'create matrix', 'free matrix', 'copy matrix', and such. It looks
> like gtk. My main purpose was to remove the lapack dependancy/code
> duplication, so i did a minmalist patch, but there's a lot of room for
> improvement here, by switching to actual C++, and maybe use slightly more of
> eigen (convolution and other stuff).
>
> Oh, and, last thing, i'm not subscribed to [hidden email], please CC:
> me. I'll try to look at the list archive.
>
> best regards,
>
> --
>
> Thomas Capricelli
>
> http://www.freehackers.org/thomas
>
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel
Reply | Threaded
Open this post in threaded view
|

Re: [patch] Getting rid of the lapack dependancy?

Thomas Capricelli

Ok, according to what i see, this 'refocus' code is what is now named 'sharp' in your code. I understand better now why it looks so much like gtk :-)

I've created a bug in kde bugzilla related to this patch, as requested:

https://bugs.kde.org/show_bug.cgi?id=251563

greetings,

--

Thomas Capricelli

http://www.freehackers.org/thomas

On Friday 17 September 2010 09:24:44 Gilles Caulier wrote:

> Thanks for your patch...

>

> The reason of clapack inclusion in digiKam core is due to use excelent

> refocus algorithm taken from an old gimp plugin, rewritten and

> improved a little bit to support 16 bits color depth images.

>

> http://refocus.sourceforge.net/

>

> As you can see there :

>

> http://refocus.cvs.sourceforge.net/viewvc/refocus/refocus/lib/

>

> refocus algorithm use an embeded version of clapack code as well. I

> personally included this algorithm in digiKam core and of course, this

> include clapack code.

>

> Your patch is interresting. I recommend to create an entry in KDE

> bugzilla and attach your patch as well. It always better to use

> bugzilla instead the mailing list about patch, because we save the

> story in time. Nothing is lost. I will review later, when time permit

> your patch to include it in digiKam.

>

> Thanks in advance

>

> Gilles Caulier


_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel
Reply | Threaded
Open this post in threaded view
|

Re: [patch] Getting rid of the lapack dependancy?

Gilles Caulier-4
Thanks

Gilles

2010/9/17  <[hidden email]>:

> Ok, according to what i see, this 'refocus' code is what is now named
> 'sharp' in your code. I understand better now why it looks so much like gtk
> :-)
>
> I've created a bug in kde bugzilla related to this patch, as requested:
>
> https://bugs.kde.org/show_bug.cgi?id=251563
>
> greetings,
>
> --
>
> Thomas Capricelli
>
> http://www.freehackers.org/thomas
>
> On Friday 17 September 2010 09:24:44 Gilles Caulier wrote:
>
>> Thanks for your patch...
>
>>
>
>> The reason of clapack inclusion in digiKam core is due to use excelent
>
>> refocus algorithm taken from an old gimp plugin, rewritten and
>
>> improved a little bit to support 16 bits color depth images.
>
>>
>
>> http://refocus.sourceforge.net/
>
>>
>
>> As you can see there :
>
>>
>
>> http://refocus.cvs.sourceforge.net/viewvc/refocus/refocus/lib/
>
>>
>
>> refocus algorithm use an embeded version of clapack code as well. I
>
>> personally included this algorithm in digiKam core and of course, this
>
>> include clapack code.
>
>>
>
>> Your patch is interresting. I recommend to create an entry in KDE
>
>> bugzilla and attach your patch as well. It always better to use
>
>> bugzilla instead the mailing list about patch, because we save the
>
>> story in time. Nothing is lost. I will review later, when time permit
>
>> your patch to include it in digiKam.
>
>>
>
>> Thanks in advance
>
>>
>
>> Gilles Caulier
>
> _______________________________________________
> Digikam-devel mailing list
> [hidden email]
> https://mail.kde.org/mailman/listinfo/digikam-devel
>
>
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel