Add structured SampleSet export/import

[bernalde]

Summary

• Add sampleset_table, write_samples, and read_samples for solver-independent SampleSet distribution caches.
• Write stable CSV rows with rank,state,reads,value,probability, with optional probability omission and native/reverse state ordering.
• Preserve solution metadata, sense/domain, package versions, and optional model scale/offset/variables through embedded JSON metadata or a JSON sidecar.
• Document the save/reload/replot workflow and add unit coverage for round-trips, zero reads, duplicate imported states, sidecar metadata, and reverse ordering.

Closes #93

Tests

• /home/bernalde/.juliaup/bin/julialauncher +release --project=. -e 'using QUBOTools; println("loaded")'
• /home/bernalde/.juliaup/bin/julialauncher +release --project=. -e 'using QUBOTools; mktempdir() do dir; sol = SampleSet{Float64,Int}(Sample{Float64,Int}[Sample([0, 1], 1.0, 2), Sample([0, 1], 1.0, 3)]; metadata = Dict{String,Any}("origin" => "smoke"), sense = :min, domain = :bool); path = joinpath(dir, "samples.csv"); write_samples(path, sol); dst = read_samples(path); @assert length(dst) == 1; @assert QUBOTools.reads(dst, 1) == 5; @assert QUBOTools.metadata(dst)["origin"] == "smoke"; @assert sampleset_table(dst)[1].probability == 1.0; end; println("samples smoke ok")'
• /home/bernalde/.juliaup/bin/julialauncher +release --project=. -e 'using QUBOTools; mktempdir() do dir; path = joinpath(dir, "dups.csv"); open(path, "w") do io; println(io, "rank,state,reads,value,probability"); println(io, "1,01,2,1.0,0.4"); println(io, "2,01,3,1.0,0.6"); end; dst = read_samples(path); @assert length(dst) == 1; @assert QUBOTools.state(dst, 1) == [0, 1]; @assert QUBOTools.reads(dst, 1) == 5; end; println("no metadata import ok")'
• /home/bernalde/.juliaup/bin/julialauncher +release --project=. -e 'using QUBOTools; mktempdir() do dir; sol = SampleSet{Float64,Int}(Sample{Float64,Int}[Sample([1, -1, 1], 2.0, 4)]; sense = :max, domain = :spin); path = joinpath(dir, "spin.csv"); meta = joinpath(dir, "spin.json"); write_samples(path, sol; metadata_path = meta, bit_order = :reverse); dst = read_samples(path; metadata_path = meta); @assert QUBOTools.state(dst, 1) == [1, -1, 1]; @assert QUBOTools.sense(dst) === QUBOTools.Max; @assert QUBOTools.domain(dst) === QUBOTools.SpinDomain; end; println("metadata bit order ok")'
• /home/bernalde/.juliaup/bin/julialauncher +release --project=. -e 'using Pkg; Pkg.test()' passes with 671 tests.

Notes: Pkg.test() re-resolved the temporary test environment and emitted the repo's existing Documenter warning about undocumented docstrings; the suite passed.

[bernalde]

Review of #95 (structured SampleSet export/import). I am the PR author, so GitHub only permits a COMMENT event from me; the formal verdict (approval or change request) must come from another maintainer. Based on the findings below my verdict would be REQUEST_CHANGES.

Setup: fetched origin/main and head 842eaeb, diffed against merge-base 8ef0b39. File/line totals match the PR (5 files, +594/-0). Reviewed on a clean tree at the PR head.

Intent vs issue #93: the PR delivers write_samples/read_samples/sampleset_table, the stable CSV schema, embedded/sidecar JSON metadata, and the documented save/reload workflow, which covers most of the issue's scope. Closes #93 is appropriate in principle, but see the blocking bug — a core acceptance criterion ("round-trip tests preserve states ... and domain") is not actually met for spin states, so I would not auto-close the issue on merge yet.

1. Blocking issues

• Spin SampleSets containing an all-up sample ([1,1,1]) corrupt or fail on round-trip. State serialization chooses its delimiter from sample values, not the domain, so an all-+1 spin state is written concatenated (111) and then parsed as the integer 111 on read. This either throws All samples must have states of equal length (mixed lengths) or silently turns [1,1,1] into [111]. Details and a reproduction are in the inline comment on src/library/solution/table.jl. This breaks the issue's "preserve states and domain" acceptance criterion and is untested. I would not merge this until it is fixed and covered by a test.

2. Nonblocking issues

• read_samples(...; bit_order=...) is silently overridden by the file's recorded bit_order whenever metadata is present, making the documented argument a no-op for any file with embedded/sidecar metadata. See inline comment.
• Model scale/offset/variables are written (via the model kwarg) but never restored by read_samples; only solution metadata is read back. The values are currently write-only and untested on the read path. See inline comment.
• Files written with metadata_path (sidecar) carry no embedded frame, so reading them WITHOUT passing metadata_path silently yields the default frame (sense = Min, domain = BoolDomain) and default bit_order, with no warning. For a reverse-ordered or spin file this silently corrupts the recovered states/frame. Consider always embedding a minimal frame, or warning when a metadata-less file is read.
• read_samples always returns SampleSet{Float64,Int} regardless of the source SampleSet's parametrization (e.g. {Float32,Int16} widens on round-trip). Minor; worth a docstring note.

3. Questions

• Is the silent override of the bit_order argument by file metadata intended? If so, the docstring should say the recorded order always wins; if not, the explicit argument should take precedence.
• Should read_samples reconstruct (or at least surface) the persisted model scale/offset/variables, given #93 lists them as in-scope?

4. Tests run and outcomes

• julia --project=. -e 'using QUBOTools; ...' smoke load: ok.
• Targeted test_solution_sampleset_io(): 16/16 pass.
• Full test_solution() group: 141/141 pass.
• Manual edge probes: empty SampleSet round-trip ok; scientific-notation/negative values ok; all-up spin round-trip FAILS (see blocking). I did not re-run the full Pkg.test() (the PR reports 671 passing); the new code path's failure is in an untested case, so the reported count does not exercise it.

5. Merge-readiness

• Not ready as-is. One blocking correctness bug in the core round-trip path for spin data, plus the nonblocking items above. After the spin serialization is fixed (delimiter chosen by domain, not values) and covered by an all-up spin round-trip test, this should be close to mergeable. The nonblocking items do not by themselves block merge.

[bernalde]

Blocking: the delimiter is chosen from the sample's values (all(x -> x == 0 || x == 1, ψ)) rather than the solution's domain. A spin sample that is all-up ([↑,↑,↑] = [1,1,1]) has every entry in {0,1}, so it is written concatenated as 111 instead of space-separated. On read, _parse_samples_state only splits a concatenated string into per-character bits for the bool domain (line 432), so in a spin file 111 is parsed as the single integer 111.

Result: a spin SampleSet with any all-up sample either throws All samples must have states of equal length (when other samples parse to a different length) or silently round-trips [1,1,1] to [111]. Reproduced:

using QUBOTools; using QUBOTools: ↑
mktempdir() do dir
  sol = SampleSet{Float64,Int}(Sample{Float64,Int}[Sample([↑,↑,↑], -3.0, 5)]; sense=:max, domain=:spin)
  p = joinpath(dir,"sp.csv"); write_samples(p, sol)
  @show QUBOTools.state(read_samples(p), 1)  # => [111], expected [1, 1, 1]
end

Fix: choose the serialization format from domain(sol), not from the values — concatenate only for the bool domain and always space-separate spin states. Thread the domain into sampleset_table/_samples_state_string. Add a round-trip test for an all-up spin state (the current tests only cover mixed spin states, so this is missed).

[bernalde]

Addressed in 6829fab. sampleset_table now passes domain(sol) into state serialization, so spin states are always space-separated; test_solution_sampleset_io() includes an all-up spin sidecar round-trip regression.

[bernalde]

Read side of the blocking bug above: the per-character split only fires for sample_domain === 𝔹, so a concatenated spin state written by _samples_state_string is never split back into bits. Once the write side selects its delimiter by domain (always space-separating spin), this branch is consistent; until then spin states whose entries are all +1 are mis-parsed here.

[bernalde]

Addressed in 6829fab. The writer no longer emits concatenated spin states, so the bool-only compact parse branch stays consistent; the all-up spin round-trip test covers this path.

[bernalde]

Nonblocking/Question: this makes the file's recorded bit_order unconditionally override the bit_order argument passed to read_samples whenever metadata is present (get(file_metadata, "bit_order", bit_order)). The read_samples(path; bit_order=...) parameter documented in the docstring is therefore a no-op for any file carrying embedded or sidecar metadata. Either honor the explicit argument or document that the recorded order always wins.

[bernalde]

Addressed as documentation and regression coverage in 6829fab. Recorded metadata intentionally wins so files written with bit_order=:reverse restore native order; the docs now say the keyword is for metadata-less input, and the test covers the precedence.

[bernalde]

Nonblocking: model scale/offset/variables are persisted here, but read_samples only restores metadata (via _samples_solution_metadata); this model block is dropped on read and has no read-path test. #93 lists preserving model scale/offset/variables as in-scope. Since read_samples returns a SampleSet rather than a model this may be acceptable, but the values are currently write-only — worth wiring through or documenting the limitation.

[bernalde]

Addressed as documentation and coverage in 6829fab. read_samples remains a SampleSet importer and does not reconstruct a model; the docs now state that, and the test verifies model scale/offset/variables are written to the JSON metadata.

[bernalde]

Review update for #95. I am the PR author, so GitHub only permits a COMMENT event from me; the formal verdict must come from another maintainer. My substantive verdict would be REQUEST_CHANGES because of the blocking issue below. I would not merge this until the blocking issue is addressed.

Setup: fetched origin/main and origin/fix/issue-93-sampleset-io, reviewed clean head 842eaeb against merge-base 8ef0b39. Local totals match GitHub's PR totals exactly: 5 files, +594/-0.

Intent vs issue #93: the PR implements the requested public helpers, CSV schema, metadata sidecar/embedded metadata, table conversion, tests, and docs. However, the current implementation does not satisfy the core round-trip acceptance criterion for spin states.

Blocking:

• src/library/solution/table.jl:238: spin SampleSets with an all-up state corrupt on round-trip. _samples_state_string chooses the compact 011-style representation based on sample values rather than domain(sol). A spin state [1, 1, 1] is written as 111; because the reader only character-splits compact states for the bool domain, it reads back as [111]. I reproduced this locally. Fix by choosing serialization from the solution domain: concatenate only bool states and space-separate spin states. Add an all-up spin round-trip test.

Nonblocking:

• src/library/solution/table.jl:219: metadata silently overrides the explicit read_samples(...; bit_order=...) argument whenever metadata exists. Either honor the explicit argument or document that recorded metadata always wins.
• src/library/solution/table.jl:266: model scale/offset/variables are written into file metadata but are not surfaced by read_samples; only solution metadata is restored. Since #93 names these fields as in scope, either expose/document this as write-only metadata or provide a way to recover it.
• Sidecar-mode CSV files carry no embedded frame. Reading such a CSV without passing the matching metadata_path silently defaults to min/bool/native, which can corrupt spin or reverse-order files. Consider embedding a minimal frame even when using a sidecar, or warning on metadata-less reads.
• read_samples always returns SampleSet{Float64,Int}. This may be acceptable, but it should be documented because parametric source sample sets widen on import.

Questions:

• Should user-provided bit_order override metadata, or is metadata intended to be authoritative?
• Should read_samples expose the persisted model metadata, or is the sidecar intended only for external consumers?

Tests run:

• Targeted write/read smoke for duplicate bool states: passed.
• Targeted no-metadata duplicate import: passed.
• Targeted model sidecar metadata check: passed.
• Targeted all-up spin round-trip: failed as described above.
• env QUBOTOOLS_FOREIGN_TESTS=true /home/bernalde/.juliaup/bin/julialauncher +release --project=. -e 'using Pkg; Pkg.test()': passed, 674 QUBOTools tests including foreign package tests for ToQUBO, QUBODrivers, and QUBO.
• gh pr checks 95 --repo JuliaQUBO/QUBOTools.jl: all reported checks passed, including Julia 1.10/1 ubuntu, Julia 1 windows, build, documenter/deploy, and label.

Merge-readiness:

• Not ready while the all-up spin round-trip bug remains. CI passes because the current tests miss that case.

[bernalde]

Pushed commit:

• 6829fab fix: preserve all-up spin sample I/O

Main changes:

• Fixed spin SampleSet serialization to choose compact vs spaced state strings from domain(sol), so all-up spin states are written as 1 1 1 and round-trip as [1, 1, 1].
• Added regression coverage for all-up spin sidecar round-trip, metadata-recorded bit_order precedence, and model metadata persistence in the sidecar.
• Documented that sidecar metadata must be supplied on read, metadata-recorded bit_order wins over the import keyword, read_samples imports as SampleSet{Float64,Int}, and model context is recorded in JSON metadata but not reconstructed by read_samples.

Tests run:

• Targeted all-up spin round-trip repro: passed.
• Focused test_solution_sampleset_io(): 23/23 passed.
• git diff --check: passed.
• env QUBOTOOLS_FOREIGN_TESTS=true /home/bernalde/.juliaup/bin/julialauncher +release --project=. -e 'using Pkg; Pkg.test()': passed, 681/681 QUBOTools tests plus ToQUBO@0.3.1, QUBODrivers@0.5.1, and QUBO@0.5.1 foreign tests.
• /home/bernalde/.juliaup/bin/julialauncher +release --project=docs docs/make.jl --skip-deploy: passed.

Comments intentionally not addressed with behavior changes:

• I did not make explicit bit_order override recorded metadata. For files written by write_samples, recorded metadata is needed to restore native state order, so I documented and tested that recorded metadata wins.
• I did not make read_samples reconstruct a model from persisted scale/offset/variables; that would widen the return contract beyond SampleSet. The values remain in the JSON metadata and the limitation is documented.
• I did not embed metadata in sidecar-mode CSVs or warn on metadata-less reads, preserving the current pure-CSV sidecar behavior. The requirement to pass metadata_path on read is now documented.
• I did not preserve source SampleSet type parameters on import; the Float64/Int import behavior is now documented.
• The benchmark bot PR comment was informational and did not require a code change.

Remaining risks / follow-up:

• If maintainers want sidecar CSVs to remain self-describing without the sidecar, that should be a follow-up API/design decision, such as embedding a minimal frame or adding an explicit warning path.

[github-actions]

Performance Report - main vs. dev

Timings are reported for the batched benchmark operations as minimum (median) ± std, and comparison uses a 15.00% tolerance to reduce false positives on shared runners.

case	main	dev	diff
/constructors/n=128/Model/MOI/bool	666.373 μs (714.818 μs) ± 472.864 μs	687.464 μs (743.417 μs) ± 552.147 μs	invariant (3.17%)
/constructors/n=128/Model	708.623 μs (747.851 μs) ± 946.085 μs	698.369 μs (748.689 μs) ± 989.728 μs	invariant (-1.45%)
/constructors/n=2048/Model/MOI/bool	28.800 ms (30.505 ms) ± 42.210 ms	29.073 ms (30.787 ms) ± 39.898 ms	invariant (0.95%)
/constructors/n=2048/Model	29.426 ms (33.430 ms) ± 24.714 ms	29.276 ms (32.917 ms) ± 24.803 ms	invariant (-0.51%)
/constructors/n=384/Model/MOI/bool	1.996 ms (2.308 ms) ± 625.603 μs	1.890 ms (2.175 ms) ± 693.101 μs	invariant (-5.32%)
/constructors/n=384/Model	2.510 ms (2.711 ms) ± 1.568 ms	2.531 ms (2.798 ms) ± 1.741 ms	invariant (0.84%)
/conversions/n=128/form/Dict	184.593 μs (228.952 μs) ± 3.169 ms	178.784 μs (229.754 μs) ± 3.080 ms	invariant (-3.15%)
/conversions/n=128/form/Matrix/Float32	38.827 μs (45.878 μs) ± 158.146 μs	40.019 μs (50.204 μs) ± 206.556 μs	invariant (3.07%)
/conversions/n=128/form/Matrix	52.057 μs (64.034 μs) ± 3.175 ms	53.208 μs (66.268 μs) ± 3.499 ms	invariant (2.21%)
/conversions/n=128/form/SparseMatrixCSC	166.004 μs (180.376 μs) ± 222.614 μs	177.942 μs (193.381 μs) ± 250.104 μs	invariant (7.19%)
/conversions/n=128/ising/Matrix	52.207 μs (62.532 μs) ± 3.096 ms	51.817 μs (62.742 μs) ± 3.174 ms	invariant (-0.75%)
/conversions/n=128/qubo/Matrix/min	642.198 μs (694.113 μs) ± 7.735 ms	636.889 μs (696.657 μs) ± 8.000 ms	invariant (-0.83%)
/conversions/n=128/qubo/Matrix	343.196 μs (378.413 μs) ± 5.609 ms	347.953 μs (378.038 μs) ± 5.917 ms	invariant (1.39%)
/conversions/n=384/form/Dict	341.744 μs (625.477 μs) ± 5.233 ms	401.553 μs (616.660 μs) ± 3.888 ms	regression (17.50%)
/conversions/n=384/form/Matrix/Float32	174.928 μs (244.516 μs) ± 12.777 ms	171.523 μs (262.527 μs) ± 11.766 ms	invariant (-1.95%)
/conversions/n=384/form/Matrix	282.046 μs (382.745 μs) ± 19.916 ms	283.648 μs (416.359 μs) ± 19.299 ms	invariant (0.57%)
/conversions/n=384/form/SparseMatrixCSC	1.269 ms (1.384 ms) ± 8.921 ms	1.421 ms (1.559 ms) ± 9.582 ms	invariant (11.98%)
/conversions/n=384/ising/Matrix	276.627 μs (388.603 μs) ± 19.954 ms	313.002 μs (416.625 μs) ± 19.583 ms	invariant (13.15%)
/conversions/n=384/qubo/Matrix/min	6.269 ms (6.457 ms) ± 40.104 ms	6.156 ms (6.339 ms) ± 38.728 ms	invariant (-1.80%)
/conversions/n=384/qubo/Matrix	3.156 ms (3.327 ms) ± 32.263 ms	3.107 ms (3.272 ms) ± 30.590 ms	invariant (-1.56%)
/evaluation/n=128/value/dense-form	363.165 μs (396.234 μs) ± 692.988 μs	363.907 μs (395.513 μs) ± 574.227 μs	invariant (0.20%)
/evaluation/n=128/value/dict-form	266.894 μs (268.636 μs) ± 7.346 μs	270.048 μs (270.950 μs) ± 8.173 μs	invariant (1.18%)
/evaluation/n=128/value/model	51.476 μs (82.957 μs) ± 684.417 μs	51.196 μs (54.159 μs) ± 388.829 μs	invariant (-0.54%)
/evaluation/n=128/value/sparse-form	51.416 μs (83.132 μs) ± 694.488 μs	51.296 μs (79.442 μs) ± 506.765 μs	invariant (-0.23%)
/evaluation/n=384/value/dense-form	4.148 ms (4.178 ms) ± 228.578 μs	4.231 ms (4.268 ms) ± 200.773 μs	invariant (2.00%)
/evaluation/n=384/value/dict-form	505.716 μs (514.238 μs) ± 10.937 μs	511.955 μs (520.208 μs) ± 7.341 μs	invariant (1.23%)
/evaluation/n=384/value/model	173.165 μs (180.126 μs) ± 211.001 μs	167.918 μs (186.015 μs) ± 170.715 μs	invariant (-3.03%)
/evaluation/n=384/value/sparse-form	172.184 μs (179.044 μs) ± 222.001 μs	176.460 μs (186.575 μs) ± 202.065 μs	invariant (2.48%)