refactor(dataset): replace Eigen matrix with mdspan/mdarray storage - #64
Merged
Conversation
Replace Dataset's Eigen::Array/Map storage with MDArray<Scalar, dextents<int,2>>
(column-major) and a MDSpan<Scalar const, dextents<int,2>> view, removing the
Eigen dependency from dataset.hpp entirely.
Key changes:
- Storage is MDArray (owned); View is MDSpan (non-owning, used for external data)
- IsView() checks storage_.size() == 0 instead of comparing Eigen data pointers
- GetValues() returns Span<Scalar const> as before — no downstream API changes
- Add SetWeights()/Weights() for optional per-sample weight vector
- Shuffle/PermuteRows reimplemented as column-wise permutation over the flat buffer
- Normalize/Standardize operate directly on the raw column pointer + vstat
Tests updated: replace Dataset::Matrix + Eigen stream init with vector-of-columns
constructor or the raw-pointer view constructor; replace ds.Values().col(j) Eigen
expressions with ds.GetValues(int64_t{j}) spans.
…types.hpp Single source of truth for 32-byte alignment: all MDArray users (Dataset storage, interpreter dispatch buffers) now inherit alignment from the type alias definition rather than specifying it independently.
GetVariable("Y") in performance tests threw bad_optional_access because
RandomDataset generated columns named X1..Xn with no Y.
foolnotion
added a commit
that referenced
this pull request
Jul 13, 2026
refactor(dataset): replace Eigen matrix with mdspan/mdarray storage
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Dataset'sEigen::Array+Eigen::Mapstorage withMDArray<Scalar, dextents<int,2>>(owned) andMDSpan<Scalar const, dextents<int,2>>(non-owning view), both of which are already dependenciesdataset.hppentirely — Eigen is now only used in the optimizer, interpreter backends, and cost functions where it belongsIsView()now checksstorage_.size() == 0instead of comparing Eigen data pointersGetValues,Rows(),Cols(),IsView(),Shuffle,Normalize,Standardize) is preserved unchanged — no callers needed updatingNew additions
Data() -> View— exposes the rawMDSpanview for callers that want direct 2D accessSetWeights(Span<Scalar const>)/Weights() -> optional<Span<Scalar const>>— optional per-sample weight vector, laying the groundwork for weighted evaluation (PR add sample weights to Evaluator. #49)Test changes
Dataset::Matrix values(r, c); values << ...; Dataset ds(values)with thevector<vector<Scalar>>constructor or the raw-pointer view constructords.Values().col(j)Eigen column expressions withds.GetValues(int64_t{j})spansRandomDatasethelper inoperon_test.hppupdated to usevector<vector<Scalar>>columnsTest plan
ctest100%)