From 1c5fedf7afa556b782c9e4797938628a257216aa Mon Sep 17 00:00:00 2001 From: Griffin Bassman Date: Fri, 29 Sep 2023 18:57:19 -0400 Subject: [PATCH] fix: sparse weights iterator end->end (#4647) * fix: sparse weights iterator end->end * fix sparse not_null and allow more bits for sparse * validate warning * lint * use fmt * clang * use warning interface * Update settings.json * use str --- test/core.vwtest.json | 28 +++++++++++++++++++ test/train-sets/ref/sparse_load_check.stderr | 22 +++++++++++++++ test/train-sets/ref/sparse_load_check.stdout | 0 test/train-sets/ref/sparse_save_check.stderr | 23 +++++++++++++++ test/train-sets/ref/sparse_save_check.stdout | 0 .../include/vw/core/array_parameters_sparse.h | 6 ++-- vowpalwabbit/core/src/vw_validate.cc | 17 ++++++++++- 7 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 test/train-sets/ref/sparse_load_check.stderr create mode 100644 test/train-sets/ref/sparse_load_check.stdout create mode 100644 test/train-sets/ref/sparse_save_check.stderr create mode 100644 test/train-sets/ref/sparse_save_check.stdout diff --git a/test/core.vwtest.json b/test/core.vwtest.json index 020d1987f8c..cd98a5da492 100644 --- a/test/core.vwtest.json +++ b/test/core.vwtest.json @@ -6017,5 +6017,33 @@ "input_files": [ "train-sets/single_empty_lines.txt" ] + }, + { + "id": 465, + "desc": "cb_explore_adf with epsilon-greedy exploration using --sparse_weights and saving model", + "vw_command": "--cb_explore_adf --epsilon 0.1 -d train-sets/cb_test.ldf --noconstant --sparse_weights -f standard_sparse_model.vw", + "diff_files": { + "stderr": "train-sets/ref/sparse_save_check.stderr", + "stdout": "train-sets/ref/sparse_save_check.stdout" + }, + "input_files": [ + "train-sets/cb_test.ldf" + ] + }, + { + "id": 466, + "desc": "cb_explore_adf with epsilon-greedy exploration using --sparse_weights and loading model", + "vw_command": "--cb_explore_adf --epsilon 0.1 -d train-sets/cb_test.ldf --noconstant --sparse_weights -i standard_sparse_model.vw", + "diff_files": { + "stderr": "train-sets/ref/sparse_load_check.stderr", + "stdout": "train-sets/ref/sparse_load_check.stdout" + }, + "input_files": [ + "train-sets/cb_test.ldf", + "standard_sparse_model.vw" + ], + "depends_on": [ + 465 + ] } ] \ No newline at end of file diff --git a/test/train-sets/ref/sparse_load_check.stderr b/test/train-sets/ref/sparse_load_check.stderr new file mode 100644 index 00000000000..8b27fe4af70 --- /dev/null +++ b/test/train-sets/ref/sparse_load_check.stderr @@ -0,0 +1,22 @@ +using no cache +Reading datafile = train-sets/cb_test.ldf +num sources = 1 +Num weight bits = 18 +learning rate = 0.5 +initial_t = 3 +power_t = 0.5 +cb_type = mtr +Enabled learners: gd, scorer-identity, csoaa_ldf-rank, cb_adf, cb_explore_adf_greedy, shared_feature_merger +Input label = CB +Output pred = ACTION_PROBS +average since example example current current current +loss last counter weight label predict features +0.066667 0.066667 1 1.0 0:1:0.5 1:0.48 15 +0.033333 0.000000 2 2.0 1:0:0.5 1:0.95 6 + +finished run +number of examples = 3 +weighted example sum = 3.000000 +weighted label sum = 0.000000 +average loss = 0.033333 +total feature number = 27 diff --git a/test/train-sets/ref/sparse_load_check.stdout b/test/train-sets/ref/sparse_load_check.stdout new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/train-sets/ref/sparse_save_check.stderr b/test/train-sets/ref/sparse_save_check.stderr new file mode 100644 index 00000000000..b5febcde188 --- /dev/null +++ b/test/train-sets/ref/sparse_save_check.stderr @@ -0,0 +1,23 @@ +final_regressor = standard_sparse_model.vw +using no cache +Reading datafile = train-sets/cb_test.ldf +num sources = 1 +Num weight bits = 18 +learning rate = 0.5 +initial_t = 0 +power_t = 0.5 +cb_type = mtr +Enabled learners: gd, scorer-identity, csoaa_ldf-rank, cb_adf, cb_explore_adf_greedy, shared_feature_merger +Input label = CB +Output pred = ACTION_PROBS +average since example example current current current +loss last counter weight label predict features +0.666667 0.666667 1 1.0 0:1:0.5 0:0.33 15 +0.333333 0.000000 2 2.0 1:0:0.5 1:0.95 6 + +finished run +number of examples = 3 +weighted example sum = 3.000000 +weighted label sum = 0.000000 +average loss = 0.333333 +total feature number = 27 diff --git a/test/train-sets/ref/sparse_save_check.stdout b/test/train-sets/ref/sparse_save_check.stdout new file mode 100644 index 00000000000..e69de29bb2d diff --git a/vowpalwabbit/core/include/vw/core/array_parameters_sparse.h b/vowpalwabbit/core/include/vw/core/array_parameters_sparse.h index f8d1f922f78..dbebe75ed68 100644 --- a/vowpalwabbit/core/include/vw/core/array_parameters_sparse.h +++ b/vowpalwabbit/core/include/vw/core/array_parameters_sparse.h @@ -70,16 +70,16 @@ class sparse_parameters sparse_parameters& operator=(sparse_parameters&&) noexcept = delete; sparse_parameters(sparse_parameters&&) noexcept = delete; - bool not_null() { return (_weight_mask > 0 && !_map.empty()); } + bool not_null() { return (_weight_mask > 0); } VW::weight* first() { THROW_OR_RETURN("Allreduce currently not supported in sparse", nullptr); } // iterator with stride iterator begin() { return iterator(_map.begin()); } - iterator end() { return iterator(_map.begin()); } + iterator end() { return iterator(_map.end()); } // const iterator const_iterator cbegin() const { return const_iterator(_map.begin()); } - const_iterator cend() const { return const_iterator(_map.begin()); } + const_iterator cend() const { return const_iterator(_map.end()); } inline VW::weight& operator[](size_t i) { return *(get_or_default_and_get(i)); } diff --git a/vowpalwabbit/core/src/vw_validate.cc b/vowpalwabbit/core/src/vw_validate.cc index 6f7525bba6a..bf345f96d68 100644 --- a/vowpalwabbit/core/src/vw_validate.cc +++ b/vowpalwabbit/core/src/vw_validate.cc @@ -35,6 +35,21 @@ void validate_default_bits(VW::workspace& all, uint32_t local_num_bits) void validate_num_bits(VW::workspace& all) { if (all.initial_weights_config.num_bits > sizeof(size_t) * 8 - 3) - THROW("Only " << sizeof(size_t) * 8 - 3 << " or fewer bits allowed. If this is a serious limit, speak up."); + { + if (all.weights.sparse) + { + if (all.weights.sparse) + { + all.logger.err_warn( + "Bit size is {}. While this is allowed for sparse weights, it may cause an overflow and is strongly " + "recommended to use a smaller value.", + all.initial_weights_config.num_bits); + } + } + else + { + THROW("Only " << sizeof(size_t) * 8 - 3 << " or fewer bits allowed. If this is a serious limit, speak up."); + } + } } } // namespace VW