API fallback and python bindings refactoring #100
Conversation
ethanglaser
force-pushed
the
dev/eglaser/api-fallback2
branch
from
cad2bbb to
a75439b
Compare
|
Would like to clarify this PR. This PR does compile-time check to see if That is, only either LVQ/LeanVec implementation or fallback will be compiled. Does this PR also detect whether the shared library is used? |
In the current state of the PR, that is correct. I made some progress on a version that would support compilation of both when we were exploring adding the runtime check here that I could revive some parts of if desired, but I dropped these changes for now.
Not exclusively, the idea is that USE_PROPRIETARY is set during compiling in any case where the proprietary code is present (ie directly from private repo or for shared library) and then usage of these interfaces after compiling would reflect the state of the variable at compilation time |
|
We need compiling both as there would be single aplication compiled that runs on all x86 platforms and it should run shared lib on base implementation |
In scope of this PR or generally speaking? I'd be in favor of having one solution in this PR and extending in a follow-up. |
we can split to as many PRs as needed. This is overall expectation from fallback for integration - login of using or not using shared lib should be transparent. It feels like for clean setup we should consider adding API to shared lib that would allow us to query if it would be executed or not, and if not - use generic implementation instead. |
| #pragma once | ||
|
|
||
| #include "svs/quantization/lvq/lvq_concept.h" | ||
|
|
There was a problem hiding this comment.
I remember we discussed it earlier, but the file names: "lvq_concept.h" or "leanvec_concept.h" don't make sense now. Why not we combine them with "*lvq/leanvec_common.h"? If not possible, then we can rename something like lvq/leanvec_loader.h
There was a problem hiding this comment.
They are separated because lvq_common defines functionality that lvq depends on, and lvq_concept depends on lvq (see graphic in private PR for further context). It's possible these could be combined into a single file with some forward declaration if we want to, but not sure how feasible this would be.
I can rename the file from concept to loader.
|
/intelci |
Supports fallback for LVQ and LeanVec at compile time, plus migration of most python bindings to fully reside in public repo to reduce duplication. See private PR of same name for full description.