Skip to content

[BWARE] Handle hash columns in transform decoders and tighten decode metadata#2479

Open
Baunsgaard wants to merge 6 commits into
apache:mainfrom
Baunsgaard:split/decoderHash
Open

[BWARE] Handle hash columns in transform decoders and tighten decode metadata#2479
Baunsgaard wants to merge 6 commits into
apache:mainfrom
Baunsgaard:split/decoderHash

Conversation

@Baunsgaard

Copy link
Copy Markdown
Contributor

Reworks transform decoders so that hash-encoded columns survive the inverse-transform path, and tightens how decoder metadata (column indices, value mappings) is propagated and initialized.

  • Decoder: pass column-id arrays through decode/decodeFromMap so each decoder knows its own output column range
  • DecoderRecode: skip recode for hash columns, keep encoded ints passthrough; init metadata from frame consistently
  • DecoderDummycode: handle hash columns when expanding categorical bits; parallel decode path; sparse-friendly init
  • DecoderPassThrough / DecoderBin / DecoderComposite / DecoderFactory: consume the new column-id arrays from the dispatch layer
  • ColumnEncoderFeatureHash: align hash bookkeeping with the decode-side changes
  • Frame columns (HashMapToInt, StringArray): small support changes consumed by the decoder path above

@codecov

codecov Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.19512% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.48%. Comparing base (65e734e) to head (f825800).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...che/sysds/runtime/transform/decode/DecoderBin.java 81.25% 7 Missing and 2 partials ⚠️
...sds/runtime/transform/decode/DecoderDummycode.java 88.09% 4 Missing and 1 partial ⚠️
...apache/sysds/runtime/transform/decode/Decoder.java 83.33% 2 Missing and 2 partials ⚠️
.../sysds/runtime/transform/decode/DecoderRecode.java 80.00% 2 Missing ⚠️
...sysds/runtime/transform/decode/DecoderFactory.java 94.44% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2479      +/-   ##
============================================
+ Coverage     71.45%   71.48%   +0.03%     
- Complexity    48855    48903      +48     
============================================
  Files          1572     1573       +1     
  Lines        189117   189321     +204     
  Branches      37106    37141      +35     
============================================
+ Hits         135126   135333     +207     
+ Misses        43546    43537       -9     
- Partials      10445    10451       +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Baunsgaard Baunsgaard force-pushed the split/decoderHash branch from 25b1a67 to d0083b1 Compare June 9, 2026 22:16
… flow

Reworks transform decoders so that hash-encoded columns survive the
inverse-transform path, and tightens how decoder metadata (column
indices, value mappings) is propagated and initialized.

- Decoder: pass column-id arrays through decode/decodeFromMap so each
  decoder knows its own output column range
- DecoderRecode: skip recode for hash columns, keep encoded ints
  passthrough; init metadata from frame consistently
- DecoderDummycode: handle hash columns when expanding categorical
  bits; parallel decode path; sparse-friendly init
- DecoderPassThrough / DecoderBin / DecoderComposite / DecoderFactory:
  consume the new column-id arrays from the dispatch layer
- ColumnEncoderFeatureHash: align hash bookkeeping with the
  decode-side changes
- Frame columns (HashMapToInt, StringArray): small support changes
  consumed by the decoder path above
Fix two regressions in the transform decode rewrite that broke
encode/decode roundtrips on dummycoded/recoded frames:

- DecoderDummycode.decodeSparse compared 0-based sparse column indexes
  against the 1-based _clPos/_cuPos bounds used by the dense path
  (in.get(i, k-1)). This shifted every lookup by one column, so the
  first category was never matched (decoded as null) and all others
  decoded one code too low. Shift the sparse bounds and index to be
  0-based, matching the dense path.

- Restore the public no-arg constructors on DecoderComposite, DecoderBin,
  DecoderPassThrough, and DecoderRecode. Decoder is Externalizable, and
  Spark broadcasts the top-level decoder via Java serialization, which
  requires a public no-arg constructor; without it deserialization fails
  with InvalidClassException on executors.

Restores passing of TransformFrameEncodeColmapTest,
TransformFrameEncodeDecodeTest, TransformCSVFrameEncodeDecodeTest, and
TransformFrameEncodeDecodeTokenTest in single-node and Spark modes.
Cover the decoder paths touched by the hash-column and decode-metadata
changes: parallel block decode equals serial decode, the sparse and
dense dummycode decode paths agree, feature-hash columns decode through
dummycode via the magic domain-size metadata, and bin columns whose
source position is shifted by dummycoding of another column. Add exact
inverse round-trip checks for recode and dummycode to validate the
sparse binary-search decode against ground truth.
Replace the toLowerCase plus equals chain in the parse fallback with a
length-based dispatch: a single char compare for the 1-char "t"/"f"
tokens and compareToIgnoreCase for "true"/"false", matching the idiom
already used in DoubleArray.parseDouble. This avoids allocating a
lower-cased copy and rejects non-boolean strings immediately.

Restore throwing DMLRuntimeException on unparseable input. The previous
re-throw of the raw NumberFormatException changed the exception type and
broke callers such as Array.extractDouble that expect DMLRuntimeException;
the throw path is the genuinely-exceptional case, so the wrapping cost is
irrelevant there.
DecoderBin gained derived decode state (_srcCols/_dcCols) that was not
captured by writeExternal/readExternal, so a decoder broadcast to Spark
executors (which do not re-run initMetaData) would NPE on every binned
transformdecode. Serialize _srcCols/_dcCols alongside the bin boundaries,
mirroring DecoderPassThrough.

Register DecoderBin in DecoderFactory.getDecoderType/createInstance so a
composite decoder containing a bin decoder can be serialized at all;
previously it threw "Unsupported decoder type" before reaching
DecoderBin.writeExternal.

Remove the _numBins field: it was allocated but never populated, so
writeExternal wrote a zero length for every column and dropped all bin
boundaries on deserialization. The per-column bin count is recovered from
the boundary array length instead, and readExternal now allocates the
boundary arrays it reads into.

Add serialization round-trip tests (plain bin and bin-with-dummycode) plus
tests for the bin source-column offset mapping, the key==0 bin branch, and
the StringArray boolean-token getAsDouble fallback.
For feature-hashed columns the hash domain size K is stored in the single
transform meta cell, not materialized as numDistinct rows like recode/bin.
Stop overloading numDistinct=K on the hash meta column and instead pass the
set of dummycoded hash columns from DecoderFactory to the dummycode/bin/
passthrough decoders, which read K from the cell when sizing the dummycode
expansion. This keeps numDistinct semantically the meta column's own
cardinality and avoids any sentinel in the cell value.

Also trim verbose comments introduced in the transform decoders and remove
the dead commented-out _rcMapsDirect block (and its unused max accumulator)
in DecoderRecode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

1 participant