[mlpack] Loading Images, Issues

[Adithya Praveen]

 Hey Ryan,
Really appreciate the write-up about the issue. Adding the HAS_STB when compiling the file worked like a charm! You were right. Got a dopamine kick from the file finally running without the HAS_STB error :).
I used the following command to compile: "g++ cnn.cpp -o cnn `pkg-config --cflags --libs mlpack armadillo` -fopenmp -DHAS_STB"
I would love to take up the refactoring myself. Granted, you would be faster at completing the refactoring; however, it would be a good learning experience for me. If it is taking too long for me to complete, then I will definitely tell you, and you could open up an issue for the same.
My point is, I would like try out the refactoring myself. Thanks again for the cool insights!
Cheers,Adi.    On Thursday, January 9, 2020, 7:08:57 AM GMT+4, Ryan Curtin <ryan at ratml.org> wrote:  
 
 On Tue, Jan 07, 2020 at 08:59:47AM +0000, Adithya Praveen wrote:
>  Hi Ryan,
> Thanks for all the info. Super useful stuff!
> > When you compile mlpack, you'll need to make sure that the STB library
> > is found.  You can see the output when you run cmake.  Actually, CMake  
> > should download STB if it's not available---so, it sounds like maybe you
> > have installed mlpack from the package manager, and perhaps the package
> > manager you installed from did not have that support.  You might have to
> > compile it yourself then.
>
> 1. I removed all previous remnants of mlpack and compiled mlpack
> myself without a package manager, just like you suggested. And just
> like you said, CMake detected the abscence of STB_IMAGE, and
> downloaded it. However, the issue persists. Here is the output
> generated when I ran CMake, it might help identify the issue.

Okay, I see what's happening here and it could be an interesting thing
to try and solve.  When mlpack is configured and built, HAS_STB is
defined as per line 348 of CMakeLists.txt.  Whether or not the image
functionality works is dependent on this macro; you can see in
src/mlpack/core/data/load_image_impl.hpp.

But the thing is: since load_image_impl.hpp is a header file, then
whenever it is included by anything that #includes <mlpack/core.hpp>,
HAS_STB must also be defined!  So I bet that your code will work if you
add the HAS_STB macro when you compile it.

Anyway, we can do better than that.  I think the better solution
(although it's not the only one) is to wrap the parts of
load_image_impl.hpp that are dependent on the HAS_STB macro into a .cpp
file.  This will need just a little refactoring, because any template
functions must be in the _impl.hpp file.  So the trick will be removing
the non-template parts and anything that depends on STB and putting
into, e.g., load_image.cpp and then having CMake compile that file as
part of libmlpack.so.

Once that's done, the HAS_STB macro should not show up anywhere in a
.hpp file in the mlpack codebase, and the problem you're seeing when you
try to use mlpack downstream would be resolved.

Are you interested in trying to solve that?  It's ok if not---I'll just
open a Github issue for it and we can get it resolved relatively quickly
so that your problem can be fixed.

Anyway, I hope the writeup here is helpful.  Let me know if there's
anything I can clarify about it.

Thanks!

Ryan

-- 
Ryan Curtin    | "Give a man a gun and he thinks he's Superman.
ryan at ratml.org | Give him two and he thinks he's God."  - Pang
  
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://knife.lugatgt.org/pipermail/mlpack/attachments/20200109/efe3f3f5/attachment.htm>