JuliaML / LIBLINEAR.jl

LIBLINEAR bindings for Julia
Other
12 stars 10 forks source link

Update sources and add Project.toml file #18

Closed jzazo closed 5 years ago

jzazo commented 5 years ago

I updated the LIBLINEAR source files to version 2.21 and run tests. Everything seemed to work. I cannot compile windows binary files, so these need to be updated. Can you do that?

I also added a 'make clean' command before compilation in the build file.

Finally, the project uses legacy Pkg and it does not have a Project.toml file. REQUIRE file is deprecated. Therefore, I added a Project.toml file indicating dependency packages and compatibility of Julia, as indicated here. The package UUID I grabbed from this. You may want to add your email in the authors slot.

jzazo commented 5 years ago

BTW, I thought about the new functionality in my fork, and I agree it would be best if we had a single registered package with this functionality. So, if you agree, I think I could PR my fork into a separate branch of your project. That way, if anybody wants to use such branch, they would only need to add LIBLINEAR#weights for example. And we can state this functionality in the README. What do you think?

innerlee commented 5 years ago

Both are great (update version and the weights branch)! I need some time to check the changes and do the windows thing

innerlee commented 5 years ago

Hi @jzazo , there are test errors on CI, do you know the reason?

jzazo commented 5 years ago

Wow, I'm glad you managed to update!! Do you know what happened? I didn't have these problems, I could run tests and call Liblinear without issues...

innerlee commented 5 years ago

Long story.

The error appeared before when the project was upgraded to v0.7 and an issue was created https://github.com/JuliaLang/julia/issues/27751. According to Keno's comment, it might be calling the library incorrectly when passing memories.

So the first try was to disable the most complex interacting function: linear_print and that solves the problem. The rest is how to re-enable it without segfault.

linear_print was meant to control verbosity of training via a global variable. We call the api set_print_string_function to pass this julia function linear_print to LIBLINEAR. When LIBLINEAR want to print something, it calls this julia function and passes a string to it. When linear_print want to print this string, the string might have been disposed by LIBLINEAR (I guess) partly because the print is async in julia (this is also a guess). This happens on master branch of linux and mac. The win32 CI error might be due to another issue due to @cfunction's bad support for win32 (guessing, too)

The fix was to build a new library libzz whose sole functionality is a print function which is synchronous (I guess so).

You can ping Keno if you want a correct answer rather than a story :P

jzazo commented 5 years ago

Hahaha, thanks! I will pull your changes and recommit to the weight branch after checking everything works fine (some time during the weekend).