[IvanaGyro]

Motivation

In-place blockunitensor.uniform_ and blockunitensor.normal_ didn't rotate seed while generating random numbers per block, which leads the values per block use the same sequence.

Description

• Rotated seed per block by adding one, which aligns with the current implementation of random.uniform_ and random.normal_.
• Added example usage to docs/code/python/doc_codes/guide_uniten_create_from_generator.py showing in-place and global alls on existing UniTensor instances.
• Added unit tests in pytests/unitensor_random_test.py that verify deterministic, seeded behavior for both dense and block UniTensor using normal_ and uniform_.
• Made convergence criterion Arnoldi_BK_test smaller to pass the test. The test was wrongly passed with all blocks using the same random sequence.

Testing

• Added and ran the new test file pytests/unitensor_random_test.py via pytest, and the tests passed for the seeded deterministic checks of UniTensor.normal_ and UniTensor.uniform_.
• Arnoldi_Gnd.Arnoldi_BK_test

[gemini-code-assist]

Code Review

This pull request removes the normal_ and uniform_ functions for UniTensor from the random module bindings and type stubs, as they are now implemented as in-place methods on the UniTensor class. It also adds documentation examples and comprehensive unit tests for these in-place methods on both dense and block UniTensor instances. The review feedback highlights a style guide violation regarding the use of leading underscores in new helper functions, and recommends using np.testing.assert_allclose instead of np.testing.assert_array_equal for floating-point comparisons to prevent flaky tests.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 587567f1a6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.47%. Comparing base (c7fda0a) to head (d263fbf).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1074      +/-   ##
==========================================
+ Coverage   69.43%   69.47%   +0.04%     
==========================================
  Files         209      209              
  Lines       22614    22616       +2     
  Branches       72       72              
==========================================
+ Hits        15701    15713      +12     
+ Misses       6891     6881      -10     
  Partials       22       22              

Flag	Coverage Δ
cpp	69.55% <100.00%> (+0.04%)	⬆️
python	65.01% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
C++ backend	69.55% <100.00%> (+0.04%)	⬆️
Python bindings	∅ <ø> (∅)
Python package	65.01% <ø> (ø)

Files with missing lines	Coverage Δ
src/UniTensor.cpp	93.47% <100.00%> (+5.24%)	⬆️

... and 1 file with indirect coverage changes

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7fda0a...d263fbf. Read the comment docs.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 61d36ee422

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[ianmccul]

(This was posted by Codex after review by @ianmccul - the codex attribution was missing from the original posting)

A few comments after looking at the current end state of this PR.

The Arnoldi test change

The modified Arnoldi_Gnd.Arnoldi_BK_test does not have a generally correct reference calculation. It asks Arnoldi for "LM" (largest magnitude), but obtains lambda using linalg::Max, whose complex implementation compares only the real parts, and then the test discards the imaginary part of Arnoldi's eigenvalue as well. H consists of random nonsymmetric real 9 x 9 blocks, so complex eigenvalues are entirely possible. This test therefore passes for a particular seeded spectrum rather than testing "LM" correctly. The reference should select the eigenvalue with largest std::abs(z) and compare the complex value, allowing for the usual ordering/tie considerations.

Also, explicitly setting maxiter = 10000 does not alter the test: 10000 is already the default for the UniTensor Arnoldi overload. The effective change is tightening cvg_crit from its default 1e-9 to 1e-12. The EXPECT_NEAR replacement is essentially equivalent to the previous absolute-error check.

For context, the other helper in this file, CheckResult(), is called 12 times: five which cases, four dtypes, and the k=1, k=23, and “smallest dimension” cases. Each call performs a full dense eigendecomposition. The matrix is only 25 x 25 (D = 5, giving D^2 x D^2), so the repeated diagonalization is unnecessary but not an important runtime cost. There is a more direct redundancy: ExcuteTest() calls and discards H.GetOpAsMat() immediately before CheckResult() calls it again. Thus the dense operator is constructed 24 times and diagonalized 12 times. The test named smallest_dim also still uses D = 5, so it is not testing the smallest dimension.

It would be better for the Arnoldi fixture to contain fixed data rather than depend on the precise random streams used by an unrelated randomization API test.

Scope and implementation

The current diff no longer matches the PR title/body: it does not remove the module-level UniTensor overloads. It aligns the member normal_()/uniform_() block streams with the module-level implementation, adds tests/docs, and changes the Arnoldi test. It also does not implement the normal_like/uniform_like operation requested by #456, so I do not think it closes that issue.

The duplicated implementation should be removed rather than synchronized manually. One of

UniTensor::uniform_()
random::uniform_(UniTensor&)

should forward to the other, and likewise for normal_(). The fact that these two loops had already diverged is a good demonstration of why there should only be one implementation.

Testing the distributions

Exact equality between two calls using the same seed is appropriate for testing reproducibility; assert_allclose is not needed for that comparison. However, same-seed equality alone does not verify low/high, mean/std, or even that randomization occurred. I agree that hard-coded output arrays from std::uniform_real_distribution or std::normal_distribution are not portable across standard-library implementations.

A robust alternative is to retain the exact reproducibility tests and add large-sample property tests. For example, use a dense Double UniTensor with shape [256, 256] (65536 samples), nontrivial parameters, and check the first two moments with tolerances derived from eight standard errors. A broad kurtosis check distinguishes normal from uniform without depending on exact generated values:

def assert_distribution(values, expected_mean, expected_variance,
                        variance_se_factor, expected_kurtosis):
    x = np.asarray(values).ravel()
    n = x.size

    sample_mean = x.mean()
    sample_variance = x.var(ddof=1)

    mean_tol = 8.0 * np.sqrt(expected_variance / n)
    variance_tol = (
        8.0 * expected_variance * np.sqrt(variance_se_factor / n)
    )

    assert abs(sample_mean - expected_mean) < mean_tol
    assert abs(sample_variance - expected_variance) < variance_tol

    centered = x - sample_mean
    sample_kurtosis = np.mean(centered**4) / np.mean(centered**2) ** 2
    assert abs(sample_kurtosis - expected_kurtosis) < 0.25

For uniform_(low=-3, high=7), the expected mean is 2, variance is 100/12, variance_se_factor = 0.8, and kurtosis is 1.8; also check every value is within [-3, 7]. For normal_(mean=2, std=3), the expected variance is 9, variance_se_factor = 2, and kurtosis is 3.

At this sample size those are deliberately generous tolerances, robust across different valid random streams, while still reliably detecting an ignored range, wrong scale, no-op fill, or wrong distribution family. The small block-UniTensor tests can then concentrate on exact same-seed reproducibility, valid uniform bounds, and ensuring equal-sized blocks do not receive identical streams.

[ianmccul]

Follow-up with concrete tolerances for the proposed statistical tests.

For n = 256 * 256 = 65536, using eight standard errors gives deliberately conservative, implementation-independent bounds.

For uniform_(low=-3, high=7):

• expected mean: 2
• expected variance: (7 - (-3))^2 / 12 = 100/12
• mean tolerance:

  8 * sqrt((100/12) / 65536) = 0.0902
  

• variance tolerance:

  8 * (100/12) * sqrt(0.8 / 65536) = 0.2329
  

I would round these upward and use:

assert np.all(values >= -3.0)
assert np.all(values <= 7.0)
assert abs(values.mean() - 2.0) < 0.10
assert abs(values.var(ddof=1) - 100.0 / 12.0) < 0.25
assert abs(sample_kurtosis(values) - 1.8) < 0.25

For normal_(mean=2, std=3):

• expected mean: 2
• expected variance: 9
• mean tolerance:

  8 * sqrt(9 / 65536) = 0.09375
  

• variance tolerance:

  8 * 9 * sqrt(2 / 65536) = 0.3977
  

Again rounding upward:

assert abs(values.mean() - 2.0) < 0.10
assert abs(values.var(ddof=1) - 9.0) < 0.40
assert abs(sample_kurtosis(values) - 3.0) < 0.25

where

def sample_kurtosis(values):
    x = np.asarray(values).ravel()
    centered = x - x.mean()
    return np.mean(centered**4) / np.mean(centered**2) ** 2

These tolerances are intentionally much wider than ordinary sampling fluctuations. They should be robust across standard-library RNG implementations while still failing clearly if the parameters are ignored, the fill is a no-op, the scaling is wrong, or normal and uniform distributions are confused. Exact same-seed equality should remain a separate reproducibility assertion.

— Codex

[IvanaGyro]

1. For Arnoldi test, this PR is just to let it pass.
2. PR description is updated.
3. Added distribution test with fixed seed to avoid flaky tests

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4f9e4183ec

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[ianmccul]

Review by Codex, after discussion with @ianmccul.

I reviewed this more broadly because the existing RNG implementation has important consequences for what seed + i actually means.

Findings

1. seed + block_index does not implement independent streams

The new loop calls each block with seed + i. Every CPU call then constructs a fresh std::mt19937(seed), while every CUDA call creates, seeds, and destroys a fresh cuRAND generator.

This removes the exact duplicate blocks produced by the old code, but it is still arithmetic reseeding rather than consuming one random stream. Adjacent seeds are not a defined stream-splitting operation and provide no non-overlap guarantee.

Cytnx does not currently generate blocks in parallel, so there is no reason to split the stream here. The simple and robust implementation is to seed one generator and consume it sequentially across all blocks.

2. Cytnx implicitly reseeds every random operation

include/random.hpp evaluates the process-global __static_random_device() as the default seed on every call. Each backend function then constructs a new engine from that seed:

std::mt19937 eng(seed);

or, on CUDA:

curandCreateGenerator(&gen, CURAND_RNG_PSEUDO_MT19937);
curandSetPseudoRandomGeneratorSeed(gen, seed);
...
curandDestroyGenerator(gen);

Consequently, Cytnx has no persistent RNG state, no manual_seed(), no state checkpointing, and no way to reproduce a sequence of operations by seeding once. Omitting seed does not continue an existing stream; it obtains new entropy and reconstructs the engine for every operation.

This PR fixes one visible consequence of that design but preserves and extends the underlying reseeding model.

3. Randomized SVD still reuses identical streams across symmetry sectors

In Rsvd_notruncate_BlockUT_internal, every sector calls:

Rsvd_notruncate_Tensor_internal(
    BTen, svalnum, is_U, is_vT, power_iteration, seed);

with the same seed. Rand_isometry() eventually reconstructs an engine from that seed. Equal-shaped sectors therefore receive exactly identical random probes.

Thus the stream-reuse problem is broader than UniTensor::normal_() and uniform_().

4. CUDA normal generation silently fails for odd-sized real blocks

The Double and Float CUDA paths pass in->size() directly to curandGenerateNormalDouble() or curandGenerateNormal(). For pseudorandom generators, cuRAND requires the requested normal sample count to be even and otherwise returns CURAND_STATUS_LENGTH_NOT_MULTIPLE.

Cytnx ignores every cuRAND return value. Therefore a real CUDA block with 1, 3, 5, ... elements can appear to randomize successfully without being filled. Complex blocks happen to request 2 * size, which is always even.

This is directly relevant to this PR because block UniTensors can have odd-sized blocks, but the new tests exercise CPU only.

NVIDIA documentation: https://docs.nvidia.com/cuda/developer-preview/13.4/curand/curand/group__HOST.html

5. The UniTensor implementation remains duplicated

Both of these independently implement the block loop:

UniTensor::uniform_()
random::uniform_(UniTensor&)

and likewise for normal_(). Their previous divergence caused this PR. One should forward to the other so there is only one implementation.

6. The tests do not exercise RNG-state semantics

Same-seed exact equality is appropriate for testing reproducibility. However, the new statistical tests exercise unchanged CPU marginal distributions; they do not detect implicit reseeding, stream reuse in Rsvd, CUDA odd-length failure, or whether consecutive calls advance one generator state.

The important state test should look approximately like:

manual_seed(7);
a = random(...);
b = random(...);

manual_seed(7);
c = random(...);
d = random(...);

EXPECT_EQ(a, c);
EXPECT_EQ(b, d);
EXPECT_NE(a, b);

There should also be CPU and CUDA tests for multi-block UniTensors and odd-sized real normal fills.

Recommended design

Cytnx needs a state-owning random::Generator:

• Own persistent CPU or per-device CUDA RNG state.
• Seed the default generators once from OS entropy.
• Advance generator state on every operation.
• Reseed only through explicit manual_seed(seed) or Generator::manual_seed(seed).
• Allow random operations to take an optional Generator.
• Pass the same generator sequentially through every UniTensor block; do not use seed + i.
• Use seed=None in Python instead of mutable int64_t& seed = -1 binding arguments.
• Check every cuRAND return value and handle odd normal sample counts.
• Remove the duplicate UniTensor/free-function implementations.

This is the model used by NumPy's Generator and PyTorch's Generator:

• https://numpy.org/doc/stable/reference/random/
• https://docs.pytorch.org/docs/stable/generated/torch.Generator.html

Future parallel generation

Cytnx does not currently need parallel per-block RNG streams. If block generation is parallelized later, child streams should be derived using a proper spawning, hashing, or counter-based mechanism rather than seed + worker_id. NumPy's SeedSequence::spawn model is a useful reference:

https://numpy.org/doc/stable/reference/random/parallel.html

On CUDA, the current MT19937 choice cannot use cuRAND's sequence offset facility. A counter-based generator such as Philox would be a more suitable basis if reproducible parallel substreams become necessary.

Recommendation

I do not think this should be merged as the final RNG fix. It removes one exact duplicate-stream symptom, but codifies per-block reseeding and leaves exact stream reuse in randomized SVD. The implementation should instead introduce persistent generator state and consume one stream sequentially across the blocks.

[IvanaGyro]

@ianmccul Thank you for reviewing. The points 1-5 are good and make Cytnx better, but they are out of scope of this'll PR. We can open issues for them.

For the sixth point about reproducibility, this PR tests on the current python interface by this file pytests/unitensor_random_test.py. We cannot test the reproducibility with one seed by until we implement the generator mechanism like numpy.

[pcchen]

Posted by Claude Code on behalf of @pcchen

@ianmccul @IvanaGyro We checked the points raised in the review comments above against master 04f5338e, and sorted them by what they mean for v2.0.0.

Fix before v2.0.0

Point	Status	Where to fix
ut.normal_() / ut.uniform_() give identical blocks (#679)	Confirmed, and fixed by this PR: pytests/unitensor_random_test.py goes from 1 of 5 failing to 5 of 5 passing when the src/UniTensor.cpp change is applied.	This PR
The normal_ block test does not guard the fix	test_block_unitensor_normal_same_seed_is_reproducible passes on the unfixed code. It needs the same assert not np.array_equal(blocks[0], blocks[1]) that the uniform_ test has.	This PR

Deferred to after v2.0.0

Point	Status	Where
seed + i is not independent streams; one stream should be consumed across blocks	Confirmed: block 1 of a tensor seeded with 11 is identical to block 0 of a tensor seeded with 12.	#1079
Stateful Generator, manual_seed, seed=None in Python	Confirmed that every CPU and CUDA call constructs a fresh engine from the seed.	#1079

These change seeded output and the API. They will not be ready for v2.0.0, so v2.0.0 will document that seeded output is not guaranteed stable across versions or platforms. That leaves room to make the change in a 2.x release.

Can wait

Point	Status	Where
Member and module-level implementations are duplicated	Confirmed. After this PR the two give identical output, so having one forward to the other is behaviour-neutral.	This PR if convenient, otherwise a follow-up
Rsvd passes the same seed to every symmetry sector	Confirmed. Measured on 8 blocks of 60x60 over 300 trials: the mean error equals that with distinct seeds, and for independent blocks so does the spread. For identical blocks the spread is about 3x larger. A robustness issue, not an accuracy loss.	New issue
cuRAND return values are never checked	Confirmed.	New issue
CUDA normal_ silently fails for odd-sized real blocks	Did not reproduce on a V100 with CUDA 12.9: sizes 1, 3, 5 and 9 were all filled, for Double and Float.	Same new issue as above
ArnoldiGnd.ArnoldiBKTest uses a wrong reference for "LM"	Confirmed that linalg::Max on complex input compares real parts only. Test quality; the library code is not affected.	New issue
Other Arnoldi test redundancies	Not checked.	Follow-up
The distribution tests cover cytnx.random.* on Tensor, not the UniTensor methods	Confirmed by reading the tests.	Follow-up
#456 (normal_like / uniform_like)	Not addressed by this PR, and additive, so it can land in any 2.x release.	#456

Proposal for this PR

1. Scope: this PR fixes #679 and nothing more.
It makes the member functions ut.normal_() / ut.uniform_() behave like the module-level cytnx.random.normal_(ut) / uniform_(ut). It does not introduce a new seeding convention: seed + i per block is what the module-level functions already do on master.

2. Changes requested before merge.

• Add the distinct-blocks assertion to test_block_unitensor_normal_same_seed_is_reproducible, so the normal_ half of the fix has a regression guard.
• Optional, here or in a follow-up: have the member functions forward to the module-level functions, so there is one loop instead of two.

3. Merge mechanics.

• Update the branch onto current master; it is 145 commits behind but merges cleanly.
• Squash on merge. The early commit messages describe removing the random.* UniTensor overloads, which was later reverted.
• Link Same random number seed in each block #679 as the issue this PR closes. It does not close Propose PyTorch rand_like like API uniform_like(unitensor, ...) and normal_like(unitensor, ...)  #456.

4. This is not the final RNG fix.
This PR removes the identical-blocks bug and makes the member and module-level functions agree. It does not fix the stream model. The final RNG fix (one generator consumed across blocks, and a stateful Generator) is tracked in #1079. When that fix lands, the member and module-level functions change together, once.

5. Release note for v2.0.0.
Seeded ut.normal_() / ut.uniform_() on block UniTensors return different values than in 1.1.x. Seeded output is not guaranteed stable across versions or platforms until #1079 is resolved.

6. Follow-ups, each as its own issue. We will open them.

• Rsvd passes the same seed to every symmetry sector.
• cuRAND return values are never checked.
• ArnoldiGnd.ArnoldiBKTest compares against the largest real part while asking for "LM".
• linalg.Max returns DBL_MIN for a tensor whose values are all negative. This was found while checking the Arnoldi point and is unrelated to this PR.