feat: [LAS] sparse Rademacher #4243
Conversation
| @@ -58,21 +58,21 @@ BOOST_AUTO_TEST_CASE(check_AO_same_actions_same_representation) | |||
| VW::multi_ex examples; | |||
|
|
|||
| examples.push_back(VW::read_example(vw, "shared |U b c")); | |||
There was a problem hiding this comment.
adding namespaces in these examples would make the tests fail without the changes from this PR
| @@ -811,4 +837,76 @@ BOOST_AUTO_TEST_CASE(check_spanner_with_actions_that_are_linear_combinations_of_ | |||
| } | |||
| } | |||
|
|
|||
| BOOST_AUTO_TEST_CASE(check_singular_value_sum_diff_for_diff_ranks_is_small) | |||
There was a problem hiding this comment.
this checks the scaling factor that was added in this PR, without this the diff would be > 500
There was a problem hiding this comment.
this effects the non_degenerate singular value count, since the first singular values can be huge without the scaling
| @@ -182,7 +182,7 @@ void cb_explore_adf_large_action_space<randomized_svd_impl, spanner_impl>::updat | |||
| return; | |||
| } | |||
|
|
|||
| auto non_degen = std::min(_d, static_cast<uint64_t>(number_of_non_degenerate_singular_values() + 1)); | |||
| auto non_degen = std::min(_d, static_cast<uint64_t>(number_of_non_degenerate_singular_values())); | |||
There was a problem hiding this comment.
not needed with sparse rademacher
|
|
||
| BOOST_CHECK_SMALL(preds[1].score - epsilon_ur, FLOAT_TOL); | ||
| BOOST_CHECK_EQUAL(preds[1].action, 0); | ||
| // check either 1 or 2 but not both since they are a duplicate |
There was a problem hiding this comment.
made this check and test more generic since it was too specific
| @@ -741,11 +767,11 @@ BOOST_AUTO_TEST_CASE(check_spanner_with_actions_that_are_linear_combinations_of_ | |||
|
|
|||
| examples.push_back(VW::read_example(vw, "| 1:0.1 2:0.12 3:0.13 b200:2 c500:9")); | |||
|
|
|||
| examples.push_back(VW::read_example(vw, "| a_1:0.5 a_2:0.65 a_3:0.12 a100:4 a200:33")); | |||
| examples.push_back(VW::read_example(vw, "| a_1:0.8 a_2:0.32 a_3:0.15 a100:0.2 a200:0.2")); | |||
| examples.push_back(VW::read_example(vw, "| a_1:0.1 a_2:0.25 a_3:0.12 a100:1 a200:0.1")); | |||
There was a problem hiding this comment.
toned these values down since the non degen singular values are around 7 (with non-sparse rademacher they would be around 3), and once the singular values start dwindling one of the two first actions gets selected if their volume is quite high
vowpalwabbit/core/src/reductions/cb/details/large_action/one_pass_svd_impl.cc
Outdated
Show resolved
Hide resolved
| constexpr std::array<size_t, 2> AO_triplet_constructor::index_map; | ||
| constexpr std::array<float, 4> AO_triplet_constructor::value_map; |
There was a problem hiding this comment.
you need to declare the constexpr static members at namespace scope (resolved automatically in c++17)
There was a problem hiding this comment.
and can not declare them inline because (and I quote) "inline variables are only available with c++17"
vowpalwabbit/core/src/reductions/cb/details/large_action/one_pass_svd_impl.cc
Outdated
Show resolved
Hide resolved
Sparse Rademacher
also: