always iterate in sortid order

* prepares for simplifying PackedMappings
* rename num_sparse_dims -> num_mapped_dims
* rename expect_subspaces -> expected_subspaces

6 files changed, 44 insertions, 41 deletions

diff --git a/eval/src/tests/tensor/packed_mappings/packed_mappings_test.cpp b/eval/src/tests/tensor/packed_mappings/packed_mappings_test.cpp
index 0b668986cb6..3c1c6a90521 100644
--- a/eval/src/tests/tensor/packed_mappings/packed_mappings_test.cpp
+++ b/eval/src/tests/tensor/packed_mappings/packed_mappings_test.cpp
@@ -82,17 +82,19 @@ public:
         ASSERT_TRUE(builder);
         built = builder->build_mappings();
         ASSERT_TRUE(built);
-        EXPECT_EQ(builder->num_sparse_dims(), built->num_sparse_dims());
+        EXPECT_EQ(builder->num_mapped_dims(), built->num_mapped_dims());
         EXPECT_EQ(builder->size(), built->size());
         for (size_t idx = 0; idx < built->size(); ++idx) {
-            auto got = built->address_of_subspace(idx);
+            std::vector<vespalib::stringref> got(builder->num_mapped_dims());
+            built->fill_by_sortid(idx, got);
             printf("Got address:");
             for (auto ref : got) {
                 printf(" '%s'", ref.data());
             }
+            uint32_t subspace = built->subspace_of_address(got);
             uint32_t original = builder->add_mapping_for(got);
             printf(" -> %u\n", original);
-            EXPECT_EQ(idx, original);
+            EXPECT_EQ(subspace, original);
         }
     }
 };
diff --git a/eval/src/vespa/eval/tensor/mixed/packed_mappings.h b/eval/src/vespa/eval/tensor/mixed/packed_mappings.h
index 19db3566020..44ab136f8bd 100644
--- a/eval/src/vespa/eval/tensor/mixed/packed_mappings.h
+++ b/eval/src/vespa/eval/tensor/mixed/packed_mappings.h
@@ -34,36 +34,16 @@ public:
     using InternalAddress = std::vector<uint32_t>;
 
     uint32_t size() const { return _num_mappings; }
-    uint32_t num_sparse_dims() const { return _num_dims; }
+    uint32_t num_mapped_dims() const { return _num_dims; }
 
     // returns -1 if mapping does not contain address 
-    int32_t subspace_of_address(const Address &address) const;
     int32_t subspace_of_enums(const InternalAddress &address) const;
-    int32_t sortid_of_address(const Address &address) const;
-    int32_t sortid_of_enums(const InternalAddress &address) const;
-
-    Address address_of_sortid(uint32_t internal_index) const;
-    Address address_of_subspace(uint32_t subspace_index) const;
-
-    /** returns sortid */
-    uint32_t fill_by_subspace(uint32_t subspace_index, Address &address) const;
-    uint32_t fill_by_subspace(uint32_t subspace_index, InternalAddress &address) const;
+    int32_t subspace_of_address(const Address &address) const;
 
     /** returns subspace_index */
     uint32_t fill_by_sortid(uint32_t sortid, Address &address) const;
     uint32_t fill_by_sortid(uint32_t sortid, InternalAddress &address) const;
 
-    InternalAddress enums_of_sortid(uint32_t internal_index) const;
-    InternalAddress enums_of_subspace(uint32_t subspace_index) const;
-
-    int enums_compare(const uint32_t *a, const uint32_t *b) const {
-        for (size_t i = 0; i < _num_dims; ++i) {
-            if (a[i] < b[i]) return -1;
-            if (a[i] > b[i]) return 1;
-        }
-        return 0;
-    }
-
     const PackedLabels & label_store() const { return _label_store; }
 private:
     PackedMappings(uint32_t num_dims, uint32_t num_mappings,
@@ -97,6 +77,14 @@ private:
     const ConstArrayRef<uint32_t> _int_store;
     const PackedLabels _label_store;
 
+    int enums_compare(const uint32_t *a, const uint32_t *b) const {
+        for (size_t i = 0; i < _num_dims; ++i) {
+            if (a[i] < b[i]) return -1;
+            if (a[i] > b[i]) return 1;
+        }
+        return 0;
+    }
+
     uint32_t offset_of_mapping_data(uint32_t idx) const {
         return (idx * (1 + _num_dims)) + _num_mappings;
     }
@@ -110,6 +98,19 @@ private:
     const uint32_t * ptr_of_sortid(uint32_t internal_index) const {
         return &_int_store[offset_of_mapping_data(internal_index)];
     }
+
+    int32_t sortid_of_address(const Address &address) const;
+    int32_t sortid_of_enums(const InternalAddress &address) const;
+
+    /** returns sortid */
+    uint32_t fill_by_subspace(uint32_t subspace_index, Address &address) const;
+    uint32_t fill_by_subspace(uint32_t subspace_index, InternalAddress &address) const;
+
+    InternalAddress enums_of_subspace(uint32_t subspace_index) const;
+    InternalAddress enums_of_sortid(uint32_t internal_index) const;
+
+    Address address_of_sortid(uint32_t internal_index) const;
+    Address address_of_subspace(uint32_t subspace_index) const;
 };
 
 } // namespace
diff --git a/eval/src/vespa/eval/tensor/mixed/packed_mappings_builder.h b/eval/src/vespa/eval/tensor/mixed/packed_mappings_builder.h
index 0b96a76f4c8..46bed473d06 100644
--- a/eval/src/vespa/eval/tensor/mixed/packed_mappings_builder.h
+++ b/eval/src/vespa/eval/tensor/mixed/packed_mappings_builder.h
@@ -22,8 +22,8 @@ class PackedMappingsBuilder {
 public:
     using SparseAddress = std::vector<vespalib::stringref>;
 
-    PackedMappingsBuilder(uint32_t num_sparse_dims)
-      : _num_dims(num_sparse_dims),
+    PackedMappingsBuilder(uint32_t num_mapped_dims)
+      : _num_dims(num_mapped_dims),
         _labels(),
         _mappings()
     {}
@@ -37,7 +37,7 @@ public:
     size_t extra_memory() const;
     PackedMappings target_memory(char *mem_start, char *mem_end) const;
 
-    uint32_t num_sparse_dims() const { return _num_dims; }
+    uint32_t num_mapped_dims() const { return _num_dims; }
     size_t size() const { return _mappings.size(); }
 private:
     uint32_t _num_dims;
diff --git a/eval/src/vespa/eval/tensor/mixed/packed_mixed_builder.h b/eval/src/vespa/eval/tensor/mixed/packed_mixed_builder.h
index 39d8345911d..ea8a6206244 100644
--- a/eval/src/vespa/eval/tensor/mixed/packed_mixed_builder.h
+++ b/eval/src/vespa/eval/tensor/mixed/packed_mixed_builder.h
@@ -22,13 +22,13 @@ public:
     PackedMixedBuilder(const ValueType &type,
                              size_t num_mapped_in,
                              size_t subspace_size_in,
-                             size_t expect_subspaces)
+                             size_t expected_subspaces)
       : _type(type),
         _subspace_size(subspace_size_in),
         _cells(),
         _mappings_builder(num_mapped_in)
     {
-        _cells.reserve(_subspace_size * expect_subspaces);
+        _cells.reserve(_subspace_size * expected_subspaces);
     }
 
     ~PackedMixedBuilder() override = default;
diff --git a/eval/src/vespa/eval/tensor/mixed/packed_mixed_factory.cpp b/eval/src/vespa/eval/tensor/mixed/packed_mixed_factory.cpp
index db09db48a4c..2a29cab9ce0 100644
--- a/eval/src/vespa/eval/tensor/mixed/packed_mixed_factory.cpp
+++ b/eval/src/vespa/eval/tensor/mixed/packed_mixed_factory.cpp
@@ -24,10 +24,10 @@ std::unique_ptr<ValueBuilderBase>
 PackedMixedFactory::create_value_builder_base(const ValueType &type,
                                               size_t num_mapped_in,
                                               size_t subspace_size_in,
-                                              size_t expect_subspaces) const
+                                              size_t expected_subspaces) const
 {
     return typify_invoke<1,TypifyCellType,CreatePackedMixedBuilder>(type.cell_type(),
-		type, num_mapped_in, subspace_size_in, expect_subspaces);
+                    type, num_mapped_in, subspace_size_in, expected_subspaces);
 }
 
 } // namespace
diff --git a/eval/src/vespa/eval/tensor/mixed/packed_mixed_tensor.cpp b/eval/src/vespa/eval/tensor/mixed/packed_mixed_tensor.cpp
index 472cf553c8e..0e32e1b06e7 100644
--- a/eval/src/vespa/eval/tensor/mixed/packed_mixed_tensor.cpp
+++ b/eval/src/vespa/eval/tensor/mixed/packed_mixed_tensor.cpp
@@ -15,7 +15,7 @@ private:
     std::vector<uint32_t> _full_enums;
     size_t _index;
 
-    size_t num_full_dims() const { return _mappings.num_sparse_dims(); }
+    size_t num_full_dims() const { return _mappings.num_mapped_dims(); }
     size_t num_view_dims() const { return _view_dims.size(); }
     size_t num_rest_dims() const { return num_full_dims() - num_view_dims(); }
 public:
@@ -60,7 +60,7 @@ PackedMixedTensorIndexView::next_result(const std::vector<vespalib::stringref*>
 {
     assert(addr_out.size() == num_rest_dims());
     while (_index < _mappings.size()) {
-        idx_out = _mappings.fill_by_subspace(_index++, _full_enums);
+        idx_out = _mappings.fill_by_sortid(_index++, _full_enums);
         bool couldmatch = true;
         size_t vd_idx = 0;
         size_t ao_idx = 0;
@@ -105,7 +105,7 @@ private:
     std::vector<uint32_t> _lookup_enums;
     bool _first_time;
 
-    size_t num_full_dims() const { return _mappings.num_sparse_dims(); }
+    size_t num_full_dims() const { return _mappings.num_mapped_dims(); }
 public:
     PackedMixedTensorLookup(const PackedMappings& mappings)
         : _mappings(mappings),
@@ -167,7 +167,7 @@ public:
           _full_address(),
           _index(0)
     {
-        _full_address.resize(_mappings.num_sparse_dims());
+        _full_address.resize(_mappings.num_mapped_dims());
     }
 
     void lookup(const std::vector<const vespalib::stringref*> &addr) override;
@@ -185,10 +185,10 @@ PackedMixedTensorAllMappings::lookup(const std::vector<const vespalib::stringref
 bool
 PackedMixedTensorAllMappings::next_result(const std::vector<vespalib::stringref*> &addr_out, size_t &idx_out)
 {
-    assert(addr_out.size() == _mappings.num_sparse_dims());
+    assert(addr_out.size() == _mappings.num_mapped_dims());
     while (_index < _mappings.size()) {
-        idx_out = _mappings.fill_by_subspace(_index++, _full_address);
-        for (size_t i = 0; i < _mappings.num_sparse_dims(); ++i) {
+        idx_out = _mappings.fill_by_sortid(_index++, _full_address);
+        for (size_t i = 0; i < _mappings.num_mapped_dims(); ++i) {
             *addr_out[i] = _full_address[i];
         }
         return true;
@@ -208,9 +208,9 @@ PackedMixedTensor::create_view(const std::vector<size_t> &dims) const
     }
     for (size_t i = 1; i < dims.size(); ++i) {
         assert(dims[i-1] < dims[i]);
-        assert(dims[i] < _mappings.num_sparse_dims());
+        assert(dims[i] < _mappings.num_mapped_dims());
     }
-    if (dims.size() == _mappings.num_sparse_dims()) {
+    if (dims.size() == _mappings.num_mapped_dims()) {
         return std::make_unique<PackedMixedTensorLookup>(_mappings);
     }
     return std::make_unique<PackedMixedTensorIndexView>(_mappings, dims);