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
VowpalWabbit · Sep 29, 2023 · 1c5fedf · 1c5fedf
1 parent de52303
commit 1c5fedf
Show file tree

Hide file tree

Showing 7 changed files with 92 additions and 4 deletions.
diff --git 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
+    ]
   }
 ]
diff --git a/test/train-sets/ref/sparse_load_check.stderr 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
diff --git a/test/train-sets/ref/sparse_save_check.stderr 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
diff --git 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
@@ -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