code cleanup by manuschneider · Pull Request #790 · Cytnx-dev/Cytnx

manuschneider · 2026-05-01T19:21:45Z

ensures that operator<< writes to the correct ostream
fixes python bindings
removes comments of the form // cout << ...
fixes and orders includes

ensures that operator<< writes to the correct ostream fixes python bindings removes comments of the form // cout << ... fixes and orders includes

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cba2cb15cb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

codecov · 2026-05-01T19:41:35Z

Codecov Report

❌ Patch coverage is 14.79290% with 144 lines in your changes missing coverage. Please review.
✅ Project coverage is 30.03%. Comparing base (bd413ad) to head (7586dfd).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/Symmetry.cpp	0.00%	30 Missing ⚠️
src/BlockFermionicUniTensor.cpp	0.00%	21 Missing ⚠️
src/DenseUniTensor.cpp	40.00%	2 Missing and 13 partials ⚠️
src/BlockUniTensor.cpp	40.00%	5 Missing and 7 partials ⚠️
src/stat/histogram.cpp	0.00%	11 Missing ⚠️
src/backend/Storage_base.cpp	0.00%	8 Missing ⚠️
src/LinOp.cpp	0.00%	7 Missing ⚠️
pybind/unitensor_py.cpp	0.00%	1 Missing and 5 partials ⚠️
include/backend/Scalar.hpp	0.00%	5 Missing ⚠️
src/backend/StorageImplementation.cpp	0.00%	4 Missing ⚠️
... and 14 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #790      +/-   ##
==========================================
- Coverage   30.04%   30.03%   -0.01%     
==========================================
  Files         238      239       +1     
  Lines       35418    35425       +7     
  Branches    14730    14736       +6     
==========================================
  Hits        10641    10641              
- Misses      17524    17530       +6     
- Partials     7253     7254       +1

Flag	Coverage Δ
cpp	`29.54% <14.79%> (-0.01%)`	⬇️
python	`59.41% <ø> (ø)`

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

Components	Coverage Δ
C++ backend	`31.21% <14.28%> (+<0.01%)`	⬆️
Python bindings	`17.25% <20.00%> (-0.05%)`	⬇️
Python package	`59.41% <ø> (ø)`

Files with missing lines	Coverage Δ
include/Accessor.hpp	`63.15% <ø> (ø)`
include/Bond.hpp	`63.26% <ø> (ø)`
src/Accessor.cpp	`34.05% <ø> (ø)`
src/Bond.cpp	`43.75% <ø> (ø)`
src/Generator.cpp	`58.49% <ø> (ø)`
src/Network.cpp	`0.00% <ø> (ø)`
src/Physics.cpp	`12.50% <ø> (ø)`
src/Tensor.cpp	`16.39% <ø> (ø)`
src/backend/Scalar.cpp	`95.72% <ø> (ø)`
src/backend/algo_internal_cpu/Concate_internal.cpp	`100.00% <ø> (ø)`
... and 55 more

... 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 bd413ad...7586dfd. 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.

IvanaGyro · 2026-05-03T08:07:59Z

     */
-    void print() const {
-      this->_impl->print(std::cout);
-      std::cout << std::string(" Scalar dtype: [") << Type.getname(this->_impl->_dtype)
-                << std::string("]") << std::endl;
+    void print() const { this->print(std::cout); }
+    /// @cond
+    void print(std::ostream &os) const {
+      this->_impl->print(os);
+      os << std::string(" Scalar dtype: [") << Type.getname(this->_impl->_dtype) << std::string("]")
+         << std::endl;
    }
+    /// @endcond


What is the reason to add new methods which consume ostream?

std::ostream &operator<<(std::ostream &os ...) functions call the print functions. Previously, those wrote to std::cout, no matter what the ostream in operator<< was. I fixed this. To make all IO options uniform (print(), print_...() etc), I added a version that accepts an ostream always.

I feel a better pattern is creating a new method that compose the output string, and let Scaler::std::ostream &operator<<(std::ostream &os ...) call that method if std::ostream &operator<<(std::ostream &os ...) exist. With the pattern, print() is useless and can be removed.

We could directly implement it in operator<< (and print() in impl). I outlined it in #868.

IvanaGyro · 2026-05-03T08:25:30Z

You did at least four things in one commit.

ensures that operator<< writes to the correct ostream
fixes python bindings
removes comments of the form // cout << ...
fixes and orders includes

To make a PR review by reviewers easier and more quickly, developers are asked to split their great works into multiple commit by logical feature. You can find many sources talking about this principle and try to review PRs.

5.2 Distributed Git - Contributing to a Project

Next, try to make each commit a logically separate changeset. If you can, try to make your changes digestible – don’t code for a whole weekend on five different issues and then submit them all as one massive commit on Monday. Even if you don’t commit during the weekend, use the staging area on Monday to split your work into at least one commit per issue, with a useful message per commit.

Linux kernel submitting guidelines

Separate each logical change into a separate patch.

For example, if your changes include both bug fixes and performance enhancements for a single driver, separate those changes into two or more patches. If your changes include an API update, and a new driver which uses that new API, separate those into two patches.

On the other hand, if you make a single change to numerous files, group those changes into a single patch. Thus a single logical change is contained within a single patch.

Git Contribution Guidelines

Give an explanation for the change(s) that is detailed enough so that people can judge if it is good thing to do, without reading the actual patch text to determine how well the code does what the explanation promises to do.

If your description starts to get too long, that’s a sign that you probably need to split up your commit to finer grained pieces. That being said, patches which plainly describe the things that help reviewers check the patch, and future maintainers understand the code, are the most beautiful patches. Descriptions that summarize the point in the subject well, and describe the motivation for the change, the approach taken by the change, and if relevant how this differs substantially from the prior version, are all good things to have.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d4d2e135bb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 08d7678a1a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c7407b253

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

pcchen · 2026-06-29T02:04:31Z

Code review (1 actionable finding)

This is a clean, mostly-mechanical refactor that threads an explicit std::ostream& os through the print methods so operator<< writes to the stream it's given instead of always std::cout. The core operator<< fixes (Symmetry, UniTensor, Storage) look correct. One bug:

`include/backend/Storage.hpp` — `Storage::print_info(std::ostream&)` dumps all elements instead of just the info

The new stream overload delegates to the wrong underlying method:

void print_info() const { this->_impl->print_info(std::cout); }      // info only
/// @cond
void print_info(std::ostream &os) const { this->_impl->print(os); }  // info + ALL elements

Storage_base::print(os) is print_info(os); print_elems(os);, so it prints the entire storage. The no-arg print_info() correctly calls _impl->print_info(...), but the std::ostream& overload calls _impl->print(...). A caller doing stor.print_info(myStream) on a large storage gets the full element listing instead of the three-line summary — divergent behavior between print_info() and print_info(os).

    void print_info(std::ostream &os) const { this->_impl->print_info(os); }

Checked, not bugs

SparseUniTensor.cpp:2079 tmp->print_diagram() uses the old zero-arg signature on an impl pointer, but it's inside a /* … */ block (lines 1950–2099) — dead code, no break.
Public UniTensor/Network/Scalar/Storage wrappers keep their argument-less overloads (defaulting to std::cout), so existing/test callers still compile; only the internal *_base/impl call sites changed signature, and those are updated consistently.
The pybind lambda rewrites correctly disambiguate the new overloads and preserve the scoped_ostream_redirect guard.

Posted by Claude Code on behalf of @pcchen

Storage::print_info(std::ostream&) delegated to _impl->print(), which prints both the info and the full element dump, diverging from the no-argument print_info() that prints info only. Delegate to _impl->print_info() instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

pcchen · 2026-07-01T01:53:15Z

@IvanaGyro is there any concern for this PR? Otherwise I will approve and merge this PR.

manuschneider · 2026-07-01T02:17:03Z

There was a general discussion in #868 on how to print objects. It led to the general question of how to structure the API for C++ and Python, which is a bigger design decision.

code cleanup

cba2cb1

ensures that operator<< writes to the correct ostream fixes python bindings removes comments of the form // cout << ... fixes and orders includes

manuschneider added codex Pending check/approval Issue fixed, and need feedback labels May 1, 2026

chatgpt-codex-connector Bot reviewed May 1, 2026

View reviewed changes

Comment thread src/Gncon.cpp Outdated

Comment thread include/Network.hpp Outdated

Comment thread include/Gncon.hpp Outdated

IvanaGyro reviewed May 3, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed May 3, 2026

View reviewed changes

Comment thread src/Accessor.cpp

Comment thread include/backend/Storage.hpp Outdated

chatgpt-codex-connector Bot reviewed May 5, 2026

View reviewed changes

Comment thread include/backend/Storage.hpp Outdated

manuschneider force-pushed the iostream branch from 08d7678 to 72bb749 Compare May 5, 2026 05:40

manuschneider added 2 commits May 5, 2026 13:43

fixed issues

25d3917

Merge remote-tracking branch 'origin/master' into iostream

1c7407b

manuschneider force-pushed the iostream branch from 72bb749 to 1c7407b Compare May 5, 2026 05:44

chatgpt-codex-connector Bot reviewed May 5, 2026

View reviewed changes

Comment thread include/backend/Storage.hpp Outdated

fixed issue

2bee797

manuschneider marked this pull request as draft May 6, 2026 02:11

manuschneider removed the Pending check/approval Issue fixed, and need feedback label May 6, 2026

manuschneider added 2 commits May 31, 2026 03:53

Merge remote-tracking branch 'origin/master' into iostream

7c2fbb7

Merge remote-tracking branch 'origin/master' into iostream

4f626e9

This was referenced May 31, 2026

Allow to print output to any stream #868

Open

print functions can output to any stream #869

Draft

pcchen added this to the v1.2.0 milestone Jun 1, 2026

pcchen and others added 3 commits June 29, 2026 10:15

Merge remote-tracking branch 'origin/master' into iostream

14b78a3

clang

7586dfd

Uh oh!

Conversation

manuschneider commented May 1, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

IvanaGyro May 3, 2026

Choose a reason for hiding this comment

Uh oh!

manuschneider May 3, 2026

Choose a reason for hiding this comment

Uh oh!

IvanaGyro May 31, 2026

Choose a reason for hiding this comment

Uh oh!

manuschneider May 31, 2026

Choose a reason for hiding this comment

Uh oh!

IvanaGyro commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

pcchen commented Jun 29, 2026

include/backend/Storage.hpp — Storage::print_info(std::ostream&) dumps all elements instead of just the info

Checked, not bugs

Uh oh!

pcchen commented Jul 1, 2026

Uh oh!

manuschneider commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented May 1, 2026 •

edited

Loading

IvanaGyro commented May 3, 2026 •

edited

Loading

`include/backend/Storage.hpp` — `Storage::print_info(std::ostream&)` dumps all elements instead of just the info