-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Ataymano/c wrapper fix2 #1859
Ataymano/c wrapper fix2 #1859
Changes from 5 commits
3921e94
9aaa1a7
c0f7090
8bf8ba9
a02b6a8
2f8e1dc
78eff1a
2f582f3
dfe5345
b9c334a
36bf448
f24bb6d
9c01912
8eb9868
4f6bf78
26fbaec
d72a03c
a05a80b
8b950a0
b83a541
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
|
||
#ifndef STATIC_LINK_VW | ||
#define BOOST_TEST_DYN_LINK | ||
#endif | ||
|
||
#include <boost/test/unit_test.hpp> | ||
|
||
#include "vwdll.h" | ||
#include "vw.h" | ||
|
||
template<class T> | ||
void check_weights_equal(T& first, T& second) | ||
{ | ||
auto secondIt = second.begin(); | ||
for (auto firstIt : first) | ||
{ | ||
BOOST_CHECK_EQUAL(firstIt, *secondIt); | ||
++secondIt; | ||
} | ||
BOOST_CHECK_EQUAL(secondIt == second.end(), true); | ||
} | ||
|
||
BOOST_AUTO_TEST_CASE(vw_dll_parsed_and_constructed_example_parity) | ||
{ | ||
//parse example | ||
VW_HANDLE handle1 = VW_InitializeA("-q st --noconstant --quiet"); | ||
VW_EXAMPLE example_parsed; | ||
example_parsed = VW_ReadExampleA(handle1, "1 |s p^the_man w^the w^man |t p^un_homme w^un w^homme"); | ||
|
||
//construct example | ||
VW_HANDLE handle2 = VW_InitializeA("-q st --noconstant --quiet"); | ||
VW_EXAMPLE example_constructed; | ||
auto fs = VW_InitializeFeatureSpaces(2); | ||
|
||
auto first = VW_GetFeatureSpace(fs, 0); | ||
VW_InitFeatures(first, 3); | ||
auto shash = VW_SetFeatureSpace(handle2, first, "s"); | ||
VW_SetFeature(VW_GetFeature(first, 0), VW_HashFeatureA(handle2, "p^the_man", shash), 1.0f); | ||
VW_SetFeature(VW_GetFeature(first, 1), VW_HashFeatureA(handle2, "w^the", shash), 1.0f); | ||
VW_SetFeature(VW_GetFeature(first, 2), VW_HashFeatureA(handle2, "w^man", shash), 1.0f); | ||
|
||
auto second = VW_GetFeatureSpace(fs, 1); | ||
VW_InitFeatures(second, 3); | ||
auto thash = VW_SetFeatureSpace(handle2, second, "t"); | ||
VW_SetFeature(VW_GetFeature(second, 0), VW_HashFeatureA(handle2, "p^un_homme", thash), 1.0f); | ||
VW_SetFeature(VW_GetFeature(second, 1), VW_HashFeatureA(handle2, "w^un", thash), 1.0f); | ||
VW_SetFeature(VW_GetFeature(second, 2), VW_HashFeatureA(handle2, "w^homme", thash), 1.0f); | ||
|
||
example_constructed = VW_ImportExample(handle2, "1", fs, 2); | ||
|
||
|
||
// learn both | ||
auto score_parsed = VW_Learn(handle1, example_parsed); | ||
auto score_constructed = VW_Learn(handle2, example_parsed); | ||
|
||
|
||
//check parity | ||
BOOST_CHECK_EQUAL(score_parsed, score_constructed); | ||
auto vw1 = static_cast<vw*>(handle1); | ||
auto vw2 = static_cast<vw*>(handle2); | ||
|
||
BOOST_CHECK_EQUAL(vw1->weights.sparse, vw2->weights.sparse); | ||
|
||
if (vw1->weights.sparse) { | ||
check_weights_equal(vw1->weights.sparse_weights, vw2->weights.sparse_weights); | ||
} | ||
else { | ||
check_weights_equal(vw1->weights.dense_weights, vw2->weights.dense_weights); | ||
} | ||
|
||
VW_ReleaseFeatureSpace(fs, 2); | ||
|
||
VW_Finish(handle1); | ||
VW_Finish(handle2); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -77,19 +77,30 @@ VW_DLL_MEMBER void VW_CALLING_CONV VW_Finish(VW_HANDLE handle) | |||
VW::finish(*pointer); | ||||
} | ||||
|
||||
VW_DLL_MEMBER VW_EXAMPLE VW_CALLING_CONV VW_ImportExample(VW_HANDLE handle, const char * label, VW_FEATURE_SPACE* features, size_t len) | ||||
VW_DLL_MEMBER VW_EXAMPLE VW_CALLING_CONV VW_ImportExample(VW_HANDLE handle, const char * label, VW_FEATURE_SPACE features, size_t len) | ||||
{ vw * pointer = static_cast<vw*>(handle); | ||||
VW::primitive_feature_space * f = reinterpret_cast<VW::primitive_feature_space*>( features ); | ||||
return static_cast<VW_EXAMPLE>(VW::import_example(*pointer, label, f, len)); | ||||
} | ||||
|
||||
VW_DLL_MEMBER VW_FEATURE_SPACE VW_CALLING_CONV VW_InitializeFeatureSpaces(size_t len) | ||||
{ | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these needed precisely? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In other words, what is the missing functionality amongst existing interfaces? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With existing API the only way to "construct" example - VwExample ImportExample(FeatureSpace, Label); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess the question is: What is the api? Is it vwdll.cpp? Or is it vw.h? To the extent that vwdll.cpp requires additional functions exposing vw.h that seems fine. To the extent that vw.h is missing things, I'd like to understand more deeply. Is vw.h missing things? If not, can these extra interface functions just call the vw.h interface? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My understanding that cpp api is vw.h while c one is vwdll. vowpal_wabbit/vowpalwabbit/vw.h Line 54 in 9206466
So given current vw.h code, direct manipulations with primitive_feature space is technically "using vw.h interface". vw.h can be refactored to hide its feature_space implementation details behind the interfaces, but still looks like corresponding flat c methods have to be added to vwdll. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good. So, can we make sure the added vwdll functions use functions in vw.h? Currently, it seems they are not, so this is sort-of creating an alternate interface. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, let me try. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sent the update draft, please take a look. |
||||
return static_cast<VW_FEATURE_SPACE>(new VW::primitive_feature_space[len]); | ||||
} | ||||
|
||||
VW_DLL_MEMBER VW_FEATURE_SPACE VW_CALLING_CONV VW_GetFeatureSpace(VW_FEATURE_SPACE first, size_t index) | ||||
{ | ||||
VW::primitive_feature_space* f = reinterpret_cast<VW::primitive_feature_space*>(first); | ||||
return static_cast<VW_FEATURE_SPACE>(&f[index]); | ||||
} | ||||
|
||||
VW_DLL_MEMBER VW_FEATURE_SPACE VW_CALLING_CONV VW_ExportExample(VW_HANDLE handle, VW_EXAMPLE e, size_t * plen) | ||||
{ vw* pointer = static_cast<vw*>(handle); | ||||
example* ex = static_cast<example*>(e); | ||||
return static_cast<VW_FEATURE_SPACE>(VW::export_example(*pointer, ex, *plen)); | ||||
} | ||||
|
||||
VW_DLL_MEMBER void VW_CALLING_CONV VW_ReleaseFeatureSpace(VW_FEATURE_SPACE* features, size_t len) | ||||
VW_DLL_MEMBER void VW_CALLING_CONV VW_ReleaseFeatureSpace(VW_FEATURE_SPACE features, size_t len) | ||||
{ VW::primitive_feature_space * f = reinterpret_cast<VW::primitive_feature_space*>( features ); | ||||
VW::releaseFeatureSpace(f, len); | ||||
} | ||||
|
@@ -164,6 +175,32 @@ VW_DLL_MEMBER float VW_CALLING_CONV VW_GetConfidence(VW_EXAMPLE e) | |||
{ return VW::get_confidence(static_cast<example*>(e)); | ||||
} | ||||
|
||||
VW_DLL_MEMBER size_t VW_CALLING_CONV VW_SetFeatureSpace(VW_HANDLE handle, VW_FEATURE_SPACE feature_space, const char* name) | ||||
{ VW::primitive_feature_space* f = reinterpret_cast<VW::primitive_feature_space*>(feature_space); | ||||
f->name = *name; | ||||
return VW_HashSpaceA(handle, name); | ||||
} | ||||
|
||||
VW_DLL_MEMBER void VW_CALLING_CONV VW_InitFeatures(VW_FEATURE_SPACE feature_space, size_t features_count) | ||||
{ | ||||
VW::primitive_feature_space* fs = reinterpret_cast<VW::primitive_feature_space*>(feature_space); | ||||
fs->fs = new feature[features_count]; | ||||
fs->len = features_count; | ||||
} | ||||
|
||||
VW_DLL_MEMBER VW_FEATURE VW_CALLING_CONV VW_GetFeature(VW_FEATURE_SPACE feature_space, size_t index) | ||||
{ | ||||
VW::primitive_feature_space* fs = reinterpret_cast<VW::primitive_feature_space*>(feature_space); | ||||
return &(fs->fs[index]); | ||||
} | ||||
|
||||
VW_DLL_MEMBER void VW_CALLING_CONV VW_SetFeature(VW_FEATURE f, size_t feature_hash, float value) | ||||
{ | ||||
feature* _feature = reinterpret_cast<feature*>(f); | ||||
_feature->weight_index = feature_hash; | ||||
_feature->x = value; | ||||
} | ||||
|
||||
VW_DLL_MEMBER VW_FEATURE VW_CALLING_CONV VW_GetFeatures(VW_HANDLE handle, VW_EXAMPLE e, size_t* plen) | ||||
{ vw* pointer = static_cast<vw*>(handle); | ||||
return VW::get_features(*pointer, static_cast<example*>(e), *plen); | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,10 +73,12 @@ extern "C" | |
VW_DLL_MEMBER void VW_CALLING_CONV VW_Finish(VW_HANDLE handle); | ||
|
||
VW_DLL_MEMBER VW_EXAMPLE VW_CALLING_CONV VW_ImportExample( | ||
VW_HANDLE handle, const char* label, VW_FEATURE_SPACE* features, size_t len); | ||
VW_HANDLE handle, const char* label, VW_FEATURE_SPACE features, size_t len); | ||
|
||
VW_DLL_MEMBER VW_FEATURE_SPACE VW_CALLING_CONV VW_InitializeFeatureSpaces(size_t len); | ||
VW_DLL_MEMBER VW_FEATURE_SPACE VW_CALLING_CONV VW_GetFeatureSpace(VW_FEATURE_SPACE first, size_t index); | ||
VW_DLL_MEMBER VW_FEATURE_SPACE VW_CALLING_CONV VW_ExportExample(VW_HANDLE handle, VW_EXAMPLE e, size_t* plen); | ||
VW_DLL_MEMBER void VW_CALLING_CONV VW_ReleaseFeatureSpace(VW_FEATURE_SPACE* features, size_t len); | ||
VW_DLL_MEMBER void VW_CALLING_CONV VW_ReleaseFeatureSpace(VW_FEATURE_SPACE features, size_t len); | ||
#ifdef USE_CODECVT | ||
VW_DLL_MEMBER VW_EXAMPLE VW_CALLING_CONV VW_ReadExample(VW_HANDLE handle, const char16_t* line); | ||
#endif | ||
|
@@ -100,6 +102,10 @@ extern "C" | |
VW_DLL_MEMBER const char* VW_CALLING_CONV VW_GetTag(VW_EXAMPLE e); | ||
VW_DLL_MEMBER size_t VW_CALLING_CONV VW_GetFeatureNumber(VW_EXAMPLE e); | ||
VW_DLL_MEMBER float VW_CALLING_CONV VW_GetConfidence(VW_EXAMPLE e); | ||
VW_DLL_MEMBER size_t VW_CALLING_CONV VW_SetFeatureSpace(VW_HANDLE handle, VW_FEATURE_SPACE feature_space, const char* name); | ||
VW_DLL_MEMBER void VW_CALLING_CONV VW_InitFeatures(VW_FEATURE_SPACE feature_space, size_t features_count); | ||
VW_DLL_MEMBER VW_FEATURE VW_CALLING_CONV VW_GetFeature(VW_FEATURE_SPACE feature_space, size_t index); | ||
VW_DLL_MEMBER void VW_CALLING_CONV VW_SetFeature(VW_FEATURE feature, size_t feature_hash, float value); | ||
VW_DLL_MEMBER VW_FEATURE VW_CALLING_CONV VW_GetFeatures(VW_HANDLE handle, VW_EXAMPLE e, size_t* plen); | ||
VW_DLL_MEMBER void VW_CALLING_CONV VW_ReturnFeatures(VW_FEATURE f); | ||
#ifdef USE_CODECVT | ||
|
@@ -120,7 +126,9 @@ extern "C" | |
VW_DLL_MEMBER float VW_CALLING_CONV VW_Learn(VW_HANDLE handle, VW_EXAMPLE e); | ||
VW_DLL_MEMBER float VW_CALLING_CONV VW_Predict(VW_HANDLE handle, VW_EXAMPLE e); | ||
VW_DLL_MEMBER float VW_CALLING_CONV VW_PredictCostSensitive(VW_HANDLE handle, VW_EXAMPLE e); | ||
//deprecated. Please use either VW_ReadExample for parsing, or VW_ImportExample for example construction | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not deprecated, right? |
||
VW_DLL_MEMBER void VW_CALLING_CONV VW_AddLabel(VW_EXAMPLE e, float label, float weight, float base); | ||
// deprecated. Please use either VW_ReadExample for parsing, or VW_ImportExample for example construction | ||
VW_DLL_MEMBER void VW_CALLING_CONV VW_AddStringLabel(VW_HANDLE handle, VW_EXAMPLE e, const char* label); | ||
|
||
VW_DLL_MEMBER float VW_CALLING_CONV VW_Get_Weight(VW_HANDLE handle, size_t index, size_t offset); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you be able to explain what the parse_mask is for?