Skip to content

Commit

Permalink
fix: Remove dead code, expand testing to more scenarios
Browse files Browse the repository at this point in the history
  • Loading branch information
lokitoth committed Feb 15, 2024
1 parent 01515a7 commit 9b79c0b
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 26 deletions.
21 changes: 1 addition & 20 deletions vowpalwabbit/fb_parser/src/parse_example_flatbuffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -373,30 +373,11 @@ int parser::get_namespace_index(const Namespace* ns, namespace_index& ni, VW::ex
ni = static_cast<uint8_t>(ns->name()->c_str()[0]);
return VW::experimental::error_code::success;
}
else if (flatbuffers::IsFieldPresent(ns, Namespace::VT_HASH))
else
{
ni = ns->hash();
return VW::experimental::error_code::success;
}

if (_active_collection && _active_multi_ex)
{
RETURN_ERROR_LS(status, fb_parser_name_hash_missing)
<< "Either name or hash field must be specified to get the namespace index in collection item with example "
"index "
<< _example_index << "and multi example index " << _multi_ex_index;
}
else if (_active_multi_ex)
{
RETURN_ERROR_LS(status, fb_parser_name_hash_missing)
<< "Either name or hash field must be specified to get the namespace index in multi example index "
<< _multi_ex_index;
}
else
{
RETURN_ERROR_LS(status, fb_parser_name_hash_missing)
<< "Either name or hash field must be specified to get the namespace index";
}
}

bool get_namespace_hash(VW::workspace* all, const Namespace* ns, uint64_t& hash)
Expand Down
4 changes: 4 additions & 0 deletions vowpalwabbit/fb_parser/tests/example_data_generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ class example_data_generator

static VW::rand_state create_test_random_state();

inline bool random_bool() { return rng.get_and_update_random() >= 0.5; }

inline int random_int(int min, int max) { return static_cast<int>(rng.get_and_update_random() * (max - min) + min); }

prototype_namespace_t create_namespace(std::string name, uint8_t numeric_features, uint8_t string_features);

prototype_example_t create_simple_example(uint8_t numeric_features, uint8_t string_features);
Expand Down
23 changes: 17 additions & 6 deletions vowpalwabbit/fb_parser/tests/read_span_tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ inline void verify_multi_ex(
} // namespace vwtest

template <typename T, typename FB_t = typename vwtest::fb_type<T>::type>
void create_flatbuffer_span_and_validate(VW::workspace& w, const T& prototype)
void create_flatbuffer_span_and_validate(VW::workspace& w, vwtest::example_data_generator& data_gen, const T& prototype)
{
// This is what we expect to see when we use read_span_flatbuffer, since this is intended
// to be used for inference, and we would prefer not to force consumers of the API to have
Expand All @@ -85,6 +85,11 @@ void create_flatbuffer_span_and_validate(VW::workspace& w, const T& prototype)
flatbuffers::uoffset_t size = builder.GetSize();

VW::multi_ex parsed_examples;
if (data_gen.random_bool())
{
parsed_examples.push_back(&ex_fac());
}

VW::parsers::flatbuffer::read_span_flatbuffer(&w, buffer, size, ex_fac, parsed_examples);

verify_multi_ex(w, prototype, parsed_examples);
Expand All @@ -100,7 +105,7 @@ TEST(FlatbufferParser, ReadSpanFlatbuffer_SingleExample)
vwtest::prototype_example_t prototype = {
{data_gen.create_namespace("A", 3, 4), data_gen.create_namespace("B", 2, 5)}, vwtest::simple_label(1.0f)};

create_flatbuffer_span_and_validate(*all, prototype);
create_flatbuffer_span_and_validate(*all, data_gen, prototype);
}

TEST(FlatbufferParser, ReadSpanFlatbuffer_MultiExample)
Expand All @@ -110,7 +115,7 @@ TEST(FlatbufferParser, ReadSpanFlatbuffer_MultiExample)
vwtest::example_data_generator data_gen;
vwtest::prototype_multiexample_t prototype = data_gen.create_cb_adf_example(3, 1, "tag");

create_flatbuffer_span_and_validate(*all, prototype);
create_flatbuffer_span_and_validate(*all, data_gen, prototype);
}

TEST(FlatbufferParser, ReadSpanFlatbuffer_ExampleCollectionSinglelines)
Expand All @@ -120,7 +125,7 @@ TEST(FlatbufferParser, ReadSpanFlatbuffer_ExampleCollectionSinglelines)
vwtest::example_data_generator data_gen;
vwtest::prototype_example_collection_t prototype = data_gen.create_simple_log(3, 3, 4);

create_flatbuffer_span_and_validate(*all, prototype);
create_flatbuffer_span_and_validate(*all, data_gen, prototype);
}

TEST(FlatbufferParser, ReadSpanFlatbuffer_ExampleCollectionMultiline)
Expand All @@ -130,13 +135,19 @@ TEST(FlatbufferParser, ReadSpanFlatbuffer_ExampleCollectionMultiline)
vwtest::example_data_generator data_gen;
vwtest::prototype_example_collection_t prototype = data_gen.create_cb_adf_log(1, 3, 4);

create_flatbuffer_span_and_validate(*all, prototype);
create_flatbuffer_span_and_validate(*all, data_gen, prototype);
}

template <int error_code>
void finish_flatbuffer_and_expect_error(FlatBufferBuilder& builder, Offset<fb::ExampleRoot> root, VW::workspace& w)
{
VW::example_factory_t ex_fac = [&w]() -> VW::example& { return VW::get_unused_example(&w); };
VW::example_sink_f ex_sink = [&w](VW::example& ex) { VW::finish_example(w, ex); };

Check failure on line 145 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.ubuntu-latest.vcpkg-asan-debug

conversion from ‘finish_flatbuffer_and_expect_error<7>(flatbuffers::FlatBufferBuilder&, flatbuffers::Offset<VW::parsers::flatbuffer::ExampleRoot>, VW::workspace&)::<lambda(VW::example&)>’ to non-scalar type ‘VW::example_sink_f’ {aka ‘std::function<void(std::vector<VW::example*>&&)>’} requested

Check failure on line 145 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.ubuntu-latest.vcpkg-asan-debug

conversion from ‘finish_flatbuffer_and_expect_error<8>(flatbuffers::FlatBufferBuilder&, flatbuffers::Offset<VW::parsers::flatbuffer::ExampleRoot>, VW::workspace&)::<lambda(VW::example&)>’ to non-scalar type ‘VW::example_sink_f’ {aka ‘std::function<void(std::vector<VW::example*>&&)>’} requested

Check failure on line 145 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.ubuntu-latest.vcpkg-asan-debug

conversion from ‘finish_flatbuffer_and_expect_error<12>(flatbuffers::FlatBufferBuilder&, flatbuffers::Offset<VW::parsers::flatbuffer::ExampleRoot>, VW::workspace&)::<lambda(VW::example&)>’ to non-scalar type ‘VW::example_sink_f’ {aka ‘std::function<void(std::vector<VW::example*>&&)>’} requested

Check failure on line 145 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.ubuntu-latest.vcpkg-asan-debug

conversion from ‘finish_flatbuffer_and_expect_error<13>(flatbuffers::FlatBufferBuilder&, flatbuffers::Offset<VW::parsers::flatbuffer::ExampleRoot>, VW::workspace&)::<lambda(VW::example&)>’ to non-scalar type ‘VW::example_sink_f’ {aka ‘std::function<void(std::vector<VW::example*>&&)>’} requested

Check failure on line 145 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.macos-latest.vcpkg-ubsan-debug

no viable conversion from '(lambda at /Users/runner/work/vowpal_wabbit/vowpal_wabbit/vowpalwabbit/fb_parser/tests/read_span_tests.cc:145:32)' to 'VW::example_sink_f' (aka 'function<void (vector<VW::example *> &&)>')

Check failure on line 145 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.macos-latest.vcpkg-ubsan-debug

no viable conversion from '(lambda at /Users/runner/work/vowpal_wabbit/vowpal_wabbit/vowpalwabbit/fb_parser/tests/read_span_tests.cc:145:32)' to 'VW::example_sink_f' (aka 'function<void (vector<VW::example *> &&)>')

Check failure on line 145 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.macos-latest.vcpkg-ubsan-debug

no viable conversion from '(lambda at /Users/runner/work/vowpal_wabbit/vowpal_wabbit/vowpalwabbit/fb_parser/tests/read_span_tests.cc:145:32)' to 'VW::example_sink_f' (aka 'function<void (vector<VW::example *> &&)>')

Check failure on line 145 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.macos-latest.vcpkg-ubsan-debug

no viable conversion from '(lambda at /Users/runner/work/vowpal_wabbit/vowpal_wabbit/vowpalwabbit/fb_parser/tests/read_span_tests.cc:145:32)' to 'VW::example_sink_f' (aka 'function<void (vector<VW::example *> &&)>')

Check failure on line 145 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.macos-latest.vcpkg-asan-debug

no viable conversion from '(lambda at /Users/runner/work/vowpal_wabbit/vowpal_wabbit/vowpalwabbit/fb_parser/tests/read_span_tests.cc:145:32)' to 'VW::example_sink_f' (aka 'function<void (vector<VW::example *> &&)>')

Check failure on line 145 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.macos-latest.vcpkg-asan-debug

no viable conversion from '(lambda at /Users/runner/work/vowpal_wabbit/vowpal_wabbit/vowpalwabbit/fb_parser/tests/read_span_tests.cc:145:32)' to 'VW::example_sink_f' (aka 'function<void (vector<VW::example *> &&)>')

Check failure on line 145 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.macos-latest.vcpkg-asan-debug

no viable conversion from '(lambda at /Users/runner/work/vowpal_wabbit/vowpal_wabbit/vowpalwabbit/fb_parser/tests/read_span_tests.cc:145:32)' to 'VW::example_sink_f' (aka 'function<void (vector<VW::example *> &&)>')

Check failure on line 145 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.macos-latest.vcpkg-asan-debug

no viable conversion from '(lambda at /Users/runner/work/vowpal_wabbit/vowpal_wabbit/vowpalwabbit/fb_parser/tests/read_span_tests.cc:145:32)' to 'VW::example_sink_f' (aka 'function<void (vector<VW::example *> &&)>')
if (vwtest::example_data_generator{}::random_bool())

Check failure on line 146 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.ubuntu-latest.vcpkg-asan-debug

expected ‘)’ before ‘::’ token

Check failure on line 146 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.ubuntu-latest.vcpkg-asan-debug

could not convert ‘(vwtest::example_data_generator)(<brace-enclosed initializer list>())’ from ‘vwtest::example_data_generator’ to ‘bool’

Check failure on line 146 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.macos-latest.vcpkg-ubsan-debug

value of type 'vwtest::example_data_generator' is not contextually convertible to 'bool'

Check failure on line 146 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.macos-latest.vcpkg-ubsan-debug

expected ')'

Check failure on line 146 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.macos-latest.vcpkg-asan-debug

value of type 'vwtest::example_data_generator' is not contextually convertible to 'bool'

Check failure on line 146 in vowpalwabbit/fb_parser/tests/read_span_tests.cc

View workflow job for this annotation

GitHub Actions / asan.macos-latest.vcpkg-asan-debug

expected ')'
{
// This is only valid because ex_fac is grabbing an example from the VW example pool
ex_sink = nullptr;
}

builder.FinishSizePrefixed(root);

Expand All @@ -147,7 +158,7 @@ void finish_flatbuffer_and_expect_error(FlatBufferBuilder& builder, Offset<fb::E

VW::multi_ex parsed_examples;
EXPECT_EQ(VW::parsers::flatbuffer::read_span_flatbuffer(
&w, buffer_copy.data(), buffer_copy.size(), ex_fac, parsed_examples),
&w, buffer_copy.data(), buffer_copy.size(), ex_fac, parsed_examples, ex_sink),
error_code);
}

Expand Down

0 comments on commit 9b79c0b

Please sign in to comment.