From aa0e646cab135c3372c66e2974de4dc890ad9b84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Fr=C3=BDda?= Date: Thu, 8 Jan 2026 10:23:29 +0100 Subject: [PATCH 1/2] Fix NewChunk's behavior regarding categorical cancel_sparse --- .../src/main/java/water/fvec/NewChunk.java | 23 +++++++++++++--- .../test/java/water/fvec/NewChunkTest.java | 27 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/h2o-core/src/main/java/water/fvec/NewChunk.java b/h2o-core/src/main/java/water/fvec/NewChunk.java index f61fc26d160f..bbdeb3ac359a 100644 --- a/h2o-core/src/main/java/water/fvec/NewChunk.java +++ b/h2o-core/src/main/java/water/fvec/NewChunk.java @@ -517,9 +517,26 @@ protected final boolean isCategorical(int idx) { } public void addCategorical(int e) { - if(_ms == null || _ms.len() == _sparseLen) - append2slow(); - if( e != 0 || !isSparseZero() ) { + if(_ms == null || _ms.len() == _sparseLen) { + if (2*_sparseLen > _len && isSparseZero()) { + int ids[] = new int[_id.length+1]; + System.arraycopy(_id, 0, ids, 1, _id.length); + ids[0] = -1; + // append2slow() will run cancel_sparse which will lose the information about 0 being a category. + // To be safe just set to categorical only those zeros that are surrounded by categories. + // We care only about those zeros set before cancel_sparse was called after the call all zeros + // should be set correctly as category since isSparseZero() == false + append2slow(); + for (int i = 0; i < ids.length-1; i++) { + if (_xs.isCategorical(ids[i+1]) && (i==0 || _xs.isCategorical(ids[i]))) + for (int j = ids[i]+1; j < ids[i+1]; j++) { + _xs.setCategorical(j); + } + } + } else + append2slow(); + } + if(e != 0 || !isSparseZero() ) { _ms.set(_sparseLen,e); _xs.setCategorical(_sparseLen); if(_id != null) _id[_sparseLen] = _len; diff --git a/h2o-core/src/test/java/water/fvec/NewChunkTest.java b/h2o-core/src/test/java/water/fvec/NewChunkTest.java index e47857348154..b6aba2118709 100644 --- a/h2o-core/src/test/java/water/fvec/NewChunkTest.java +++ b/h2o-core/src/test/java/water/fvec/NewChunkTest.java @@ -629,5 +629,32 @@ public void testSparseWithMissing() { nc.addNumDecompose(Double.MIN_VALUE); nc.addNumDecompose(Double.MIN_NORMAL); } + + + @Test + public void testNewChunkSparseCategoricals() { + // this is reproduction of issue that caused failure in multinode for the + // targetencoding.interaction.InteractionSupportTest.test_addFeatureInteraction_with_known_domain_is_consistent + // test + int[] data = new int[96]; + for (int i = 0; i < 19; i++) + data[i] = 0; + for (int i = 19; i < 62; i++) + data[i] = 2; + for (int i = 62; i < 86; i++) + data[i] = 7; + for (int i = 86; i < 90; i++) + data[i] = 12; + for (int i = 90; i < 94; i++) + data[i] = 0; + data[94] = 11; + data[95] = 11; + + nc = new NewChunk(av, 25); + for (int i = 0; i < data.length; i++) { + nc.addCategorical(data[i]); + } + assertTrue(nc.type() == Vec.T_CAT); + } } From 7da8fca6ab86203c2b0194890efa8a9cf79a1253 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Fr=C3=BDda?= Date: Thu, 8 Jan 2026 10:51:07 +0100 Subject: [PATCH 2/2] Formatting and check data consistency in the test --- .../src/main/java/water/fvec/NewChunk.java | 39 ++++++++++--------- .../test/java/water/fvec/NewChunkTest.java | 7 +++- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/h2o-core/src/main/java/water/fvec/NewChunk.java b/h2o-core/src/main/java/water/fvec/NewChunk.java index bbdeb3ac359a..b7a4e6b525ff 100644 --- a/h2o-core/src/main/java/water/fvec/NewChunk.java +++ b/h2o-core/src/main/java/water/fvec/NewChunk.java @@ -517,33 +517,34 @@ protected final boolean isCategorical(int idx) { } public void addCategorical(int e) { - if(_ms == null || _ms.len() == _sparseLen) { - if (2*_sparseLen > _len && isSparseZero()) { - int ids[] = new int[_id.length+1]; - System.arraycopy(_id, 0, ids, 1, _id.length); - ids[0] = -1; - // append2slow() will run cancel_sparse which will lose the information about 0 being a category. - // To be safe just set to categorical only those zeros that are surrounded by categories. - // We care only about those zeros set before cancel_sparse was called after the call all zeros - // should be set correctly as category since isSparseZero() == false - append2slow(); - for (int i = 0; i < ids.length-1; i++) { - if (_xs.isCategorical(ids[i+1]) && (i==0 || _xs.isCategorical(ids[i]))) - for (int j = ids[i]+1; j < ids[i+1]; j++) { - _xs.setCategorical(j); - } - } + if (_ms == null || _ms.len() == _sparseLen) { + if (2 * _sparseLen > _len && isSparseZero()) { + int ids[] = new int[_id.length + 1]; + System.arraycopy(_id, 0, ids, 1, _id.length); + ids[0] = -1; + // append2slow() will run cancel_sparse which will lose the information about 0 being a category. + // To be safe just set to categorical only those zeros that are surrounded by categories. + // We care only about those zeros set before cancel_sparse was called after the call all zeros + // should be set correctly as category since isSparseZero() == false. + append2slow(); + for (int i = 0; i < ids.length - 1; i++) { + if (_xs.isCategorical(ids[i + 1]) && (i == 0 || _xs.isCategorical(ids[i]))) + for (int j = ids[i] + 1; j < ids[i + 1]; j++) { + _xs.setCategorical(j); + } + } } else append2slow(); } - if(e != 0 || !isSparseZero() ) { - _ms.set(_sparseLen,e); + if (e != 0 || !isSparseZero()) { + _ms.set(_sparseLen, e); _xs.setCategorical(_sparseLen); - if(_id != null) _id[_sparseLen] = _len; + if (_id != null) _id[_sparseLen] = _len; ++_sparseLen; } ++_len; } + public void addNA() { if(!_sparseNA) { if (isString()) { diff --git a/h2o-core/src/test/java/water/fvec/NewChunkTest.java b/h2o-core/src/test/java/water/fvec/NewChunkTest.java index b6aba2118709..ea819d0488b7 100644 --- a/h2o-core/src/test/java/water/fvec/NewChunkTest.java +++ b/h2o-core/src/test/java/water/fvec/NewChunkTest.java @@ -630,7 +630,6 @@ public void testSparseWithMissing() { nc.addNumDecompose(Double.MIN_NORMAL); } - @Test public void testNewChunkSparseCategoricals() { // this is reproduction of issue that caused failure in multinode for the @@ -654,7 +653,11 @@ public void testNewChunkSparseCategoricals() { for (int i = 0; i < data.length; i++) { nc.addCategorical(data[i]); } - assertTrue(nc.type() == Vec.T_CAT); + assertEquals(Vec.T_CAT, nc.type()); + Chunk c = nc.new_close(); + for (int i = 0; i < data.length; i++) { + assertEquals(data[i], (int)c.atd(i)); + } } }