Skip to content

Commit 48bce00

Browse files
committed
docs: add branch comparison analysis (feat/ds-lcao-subspace-unified vs dyzheng/lcao_solver_subspace)
1 parent 21c1ba0 commit 48bce00

1 file changed

Lines changed: 72 additions & 0 deletions

File tree

BRANCH_COMPARISON.md

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
# Branch Comparison: feat/ds-lcao-subspace-accel vs dyzheng/lcao_solver_subspace
2+
3+
Date: 2026-07-24
4+
Base: merge-base `b9669d3757`
5+
6+
## Summary
7+
8+
| | Current branch `feat/ds-lcao-subspace-unified` | dyzheng `lcao_solver_subspace` |
9+
|---|---|---|
10+
| Unique commits | **120** | **41** (2 own + 39 upstream PRs) |
11+
| gga_grad=3 | Complete (5 commits) | Missing entirely |
12+
| DeltaSpin sign convention | Unified (`H_DS = +lambda.sigma`) | Old (wrong for nspin=4) |
13+
| cal_escon gate | Fixed (empty-check guard) | Bug (`is_Mi_converged` gate) |
14+
| LambdaSolver factory | Not present | BFGS/ChiGuided/Subspace/FDCG |
15+
| HSolverLCAOSubspace | Not present (has lcao_subspace.cpp) | General-purpose for MD/relax |
16+
17+
## dyzheng's 2 Own Commits
18+
19+
### 1. `d6b70003e` - Feature: Add LCAO subspace solver for SCF acceleration (~2527 lines)
20+
- `HSolverLCAOSubspace` class for general subspace diagonalization
21+
- `LambdaSolver` factory with 4 solvers (BFGS, ChiGuided, Subspace, FDCG)
22+
- `cal_PI_sub` + `B_I_data`/`BI_AdjacentData` in dspin_lcao
23+
- New parameters: `lcao_subspace_persistent`, `lcao_subspace_clear_thr`
24+
25+
### 2. `065a84c8d` - Fix: build_subspace_lcao ZGEMM dimension (~137 lines)
26+
- Correct LCAO matrix dimensions (nlocal x nbands) instead of PW dimensions
27+
- `build_subspace_lcao()` for MD/relax subspace cache initialization
28+
29+
## Critical Issues in dyzheng's Branch
30+
31+
### Issue 1: gga_grad=3 Completely Missing
32+
Current branch has 5 commits implementing full gga_grad functionality:
33+
- `396ff46b1` gga_grad parameter for non-collinear GGA gradient methods
34+
- `8d9ff5129` Unify gga_grad=1 to use mag_part
35+
- `b5b289858` gga_grad=3 SF transform in libxc path (default=3)
36+
- `306bb14cd` Stress validation for gga_grad=3
37+
- `21c1ba060` Remove nspin=4 guard on gint_precision
38+
39+
dyzheng has none of these. Not in upstream either.
40+
41+
### Issue 2: DeltaSpin Energy Correction Bugs
42+
43+
**(a) nspin=4 off-diagonal sign error** (`dspin_lcao.cpp` cal_coeff_lambda):
44+
45+
| | H_{up,dn} | H_{dn,up} |
46+
|---|---|---|
47+
| Current (fixed) | `lambda_x - i*lambda_y` | `lambda_x + i*lambda_y` |
48+
| dyzheng (old) | `lambda_x + i*lambda_y` | `lambda_x - i*lambda_y` |
49+
50+
Fixed in current branch by commit `de96b2753`. dyzheng still uses old convention, **nspin=4 results are incorrect**.
51+
52+
**(b) cal_escon() `is_Mi_converged` gate**:
53+
54+
dyzheng:
55+
```cpp
56+
if (!this->is_Mi_converged) { return 0.0; } // returns 0 before convergence
57+
```
58+
59+
Current branch:
60+
```cpp
61+
if (this->lambda_.empty() || this->Mi_.empty()) { return 0.0; } // safe empty-check
62+
```
63+
64+
Old code skips energy correction before convergence, causing **energy discontinuity**.
65+
66+
## Recommendation
67+
68+
**Do not directly merge** dyzheng's `lcao_solver_subspace` branch.
69+
70+
**Cherry-pick candidates** (requires sign/escon fixes first):
71+
- `LambdaSolver` factory pattern (FDCG solver is more robust than BFGS)
72+
- `build_subspace_lcao()` for MD/relax subspace cache (current branch only uses subspace inside DeltaSpin lambda loop)

0 commit comments

Comments
 (0)