Skip to content

DMRG major update - #119

Merged
Ooolab merged 6 commits into
ALPSim:masterfrom
afeiguin:master
Aug 9, 2026
Merged

DMRG major update#119
Ooolab merged 6 commits into
ALPSim:masterfrom
afeiguin:master

Conversation

@afeiguin

@afeiguin afeiguin commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

New optimizations, major speedup, and new methods that will facilitate onboarding other DMRG derivatives

Type of change

  • Bug fix
  • [X ] New feature or algorithm
  • Documentation or tutorial improvement
  • Build system / dependency update
  • Refactor (no behaviour change)
  • Other:

Testing

  • [ X] Existing tests pass (ctest --output-on-failure)
  • New tests added for changed behaviour
  • [X ] Results verified against known reference values or published results
  • Tested on Linux
  • [ X] Tested on macOS

For simulation code changes

  • Physical results match expected behaviour (describe briefly below)
  • Parameter files / input format changes are documented
  • Relevant tutorial or documentation updated

Checklist

  • CI passes (build + tests on all platforms)
  • [X ] No new compiler warnings (-Wall -Wextra)
  • [X ] Code follows the style of the surrounding code
  • [ X] Commit messages are descriptive and in the imperative mood

Related issues

@marcusr2ML
marcusr2ML requested review from Ooolab and marcusr2ML August 7, 2026 16:24

@marcusr2ML marcusr2ML left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tutorials all run. DMRG-05 scripts were moving slower than old code at times it seems. There are spin-1/2's on the boundary too and so I think this throws off what the tutorial suggests should happen vs the plots we get. ALl other tutorials run faster, maybe this should be explored

I also had Claude check over the code and he flagged the issue below. Should be a quick fix if it is anything at all:

signal_emit(std::string) — undefined behavior (system.h:915)

int n = this->get_signal_id(signal_id);
if(n > 0) return this->signal_emit(n);
// no return when n <= 0

Control falls off the end of a non-void function. n > 0 should also be
n >= 0SYSTEM_SIGNAL_GS is pushed first in init_signals()
(system.h:237), so it's index 0, meaning emitting it by name takes the
broken path. get_signal_id() also returns -1 for an unknown name,
which isn't handled.

It doesn't crash — it returns a garbage value that changes with build
flags. I reproduced the same code shape with clang: 

┌───────┬─────────┬───────────────────────┐
│ build │ returns │ signal actually fired │
├───────┼─────────┼───────────────────────┤
│ -O0   │ false   │ no                    │
├───────┼─────────┼───────────────────────┤
│ -O3   │ true    │ no                    │
└───────┴─────────┴───────────────────────┘

That matters because callers use the result as
if (signal_emit(...)) measure(); — so a garbage true proceeds as if the
signal ran, and a garbage false skips the measurement. No error either way.

Fix: 

int n = this->get_signal_id(signal_id);
if(n < 0) return true;   // unknown signal: nothing vetoed
return this->signal_emit(static_cast<size_t>(n));

Not biting yet — nothing in-tree uses the string overload, and as an
uninstantiated template it's never checked, so no -Wreturn-type warning
appears in the build.

@afeiguin

afeiguin commented Aug 8, 2026 via email

Copy link
Copy Markdown
Contributor Author

@marcusr2ML
marcusr2ML dismissed their stale review August 8, 2026 21:40

A known bug that does not affect the results of the tutorial's simulations. Approving then reporting the bug as an issue gets things moving along faster.

@marcusr2ML marcusr2ML left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproduces expected results from tutorials. Fixed prior issues, like violating sum rules and converges faster.

@Ooolab
Ooolab merged commit eb6414a into ALPSim:master Aug 9, 2026
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants