Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does not build #9

Closed
ijsnow opened this issue Jan 21, 2021 · 11 comments · Fixed by #10
Closed

Does not build #9

ijsnow opened this issue Jan 21, 2021 · 11 comments · Fixed by #10

Comments

@ijsnow
Copy link

ijsnow commented Jan 21, 2021

I'm trying to run the examples and it seems like the project doesn't build at the moment. The compiler is reporting a few places where what appears to be a private serde module is being used. Did serde update and remove that export? Or am I missing something in order to import private modules?

error[E0603]: module `export` is private
   --> lsh-rs/src/hash.rs:8:12
    |
8   | use serde::export::PhantomData;
    |            ^^^^^^ private module
    |
note: the module `export` is defined here
   --> /Users/isaac/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.120/src/lib.rs:275:5
    |
275 | use self::__private as export;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0603]: module `export` is private
   --> lsh-rs/src/table/sqlite.rs:9:12
    |
9   | use serde::export::PhantomData;
    |            ^^^^^^ private module
    |
note: the module `export` is defined here
   --> /Users/isaac/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.120/src/lib.rs:275:5
    |
275 | use self::__private as export;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0603]: module `export` is private
   --> lsh-rs/src/data.rs:4:12
    |
4   | use serde::export::fmt::{Debug, Display};
    |            ^^^^^^ private module
    |
note: the module `export` is defined here
   --> /Users/isaac/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.120/src/lib.rs:275:5
    |
275 | use self::__private as export;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
@ritchie46
Copy link
Owner

Hi.. Could you tell me which example you are trying to run? I notice that the blas dependencies can be troublesome.

@ijsnow
Copy link
Author

ijsnow commented Jan 27, 2021

Sorry, coulda been more specific. I get the error when running make build in examples/neural-network

@ritchie46
Copy link
Owner

I believe the examples are not up to date anymore indeed. The lsh-rs library still builds on my side (I hope on yours as well). However the examples don't work anymore because my blas libraries don't compile. At the moment I am too busy too work that out. But as the main library still builds I believe it should work with updating some dependencies / toggling off blas. If you believe there are any issues with the main library let me know, I'll figure it out.

@bwindsor22
Copy link
Contributor

Also seeing this error when I add lsh-rs = "0.4.0" to cargo.toml and run the install.

@bwindsor22
Copy link
Contributor

bwindsor22 commented Apr 26, 2021

Hey, so I managed to compile by pulling the code out of lsh-rs/lsh-rs/src/ and copying into my src, with some module changes.. you can see this commit here if it's helpful bwindsor22/thistle@477cd93

As an aside, for vector similarity lookup, you might also check out granne, hnswlib-rs, and hnsw libraries

@bwindsor22
Copy link
Contributor

bwindsor22 commented Apr 26, 2021

@ritchie46 thanks for the comment on blas, was helpful -- thoughts on shipping the examples separately? Something like Cargo.toml -> RemoveMe.Cargo.toml for a quick fix

@bwindsor22
Copy link
Contributor

@ritchie46 err, looking back, some of the errors that @ijsnow saw were things I fixed locally -- e.g. changing this to std::marker::PhantomData

8 | use serde::export::PhantomData;
| ^^^^^^ private module

@bwindsor22
Copy link
Contributor

bwindsor22 commented Apr 26, 2021

... however when git clone and cargo make locally, I do get the blas error. So I think the blas error is an issue, but as I can get the code to compile without solving it, it might be possible to ship a working version of this lib only by quarantining the blas bit and separately solving the serde errors ijsnow was seeing

@ritchie46
Copy link
Owner

@ritchie46 err, looking back, some of the errors that @ijsnow saw were things I fixed locally -- e.g. changing this to std::marker::PhantomData

8 | use serde::export::PhantomData;
| ^^^^^^ private module

Yes, that's definitly a good one. Could you make a PR for that fix?

@ritchie46
Copy link
Owner

uarantining the blas bit and

This make sense, BLAS is only for squeezing out max performance, but should definitely be opt in.

@bwindsor22
Copy link
Contributor

Great, submitted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants