Clarify Chimera coordinate convention

[bernalde]

Summary

• Clarifies that Chimera coordinate labels use row/column tile coordinates and the dwave-networkx (i, j, u, k) shore convention.
• Documents that o = 0 is the vertical inter-tile shore and o = 1 is the horizontal inter-tile shore.
• Notes that the legacy HFS/alex1770-style writer emits these labels unchanged, preserving the documented row/column, shore-index convention.

Validation

• Compared the current graph behavior against the pinned local dwave-networkx==0.8.18 package from CondaPkg; the existing implementation already matches dwave-networkx.
• julia --project=. -e 'include("test/chimera.jl")' passed.
• julia --project=. -e 'using Pkg; Pkg.test()' passed. The D-Wave cloud test path skipped because DWAVE_API_TOKEN is not set locally.

Branch Hygiene

• Base branch: main
• Source branch point: origin/main at 1293a76
• Head branch: fix/issue-47-chimera-coordinate-convention
• Stacked status: not stacked; branch was created from current main
• Prerequisite PRs: none; PR Add Pegasus, Zephyr, and working-graph topology support #46 is already merged

Closes #47

[bernalde]

Formal verdict: APPROVE. Submitted as COMMENT because this account authored the PR, so GitHub will not accept a self-approval.

Blocking issues: none.

Nonblocking issues: none.

Questions: none.

Tests run and outcomes:

• Verified local merge-base diff totals match GitHub: 1 file changed, 6 insertions, 2 deletions.
• julia --project=. -e 'include("test/chimera.jl")' passed in a clean PR worktree.
• julia --project=. -e 'using Pkg; Pkg.test()' passed in a clean PR worktree; the live D-Wave optimizer tests skipped because DWAVE_API_TOKEN is not set locally.
• Verified the Chimera edge convention against both dwave-networkx==0.8.18 and DWave.Chimera.
• GitHub CI is green across all 4 matrix jobs.

Merge-readiness: merge-ready from this review. PR #49 is non-draft, merge state is CLEAN, and all checks are successful.

[bernalde]

Review of #49 (clarify Chimera coordinate convention, Closes #47).

Note on event type: I am the PR author, so GitHub only permits a COMMENT review. A formal APPROVE / REQUEST_CHANGES must come from another maintainer. There are no blocking issues; the items below are nonblocking.

Verification of the central claim

The PR's premise - that the implementation already matches dwave-networkx and only the docstring prose needed alignment - checks out. I verified it empirically against the pinned dwave-networkx==0.8.18 rather than by reading: for Chimera(2,3) the wrapper graph (48 vertices, 124 edges) is equal to dwave_networkx.chimera_graph(2,3,4; coordinates=true) under identity labels x=i, y=j, o=u, i=k (and not equal under o <-> 1-o or x <-> y). The current _chimera_graph couples o=0 along x and o=1 along y, which matches both the (unchanged) docstring edge rules and dnx; test/chimera.jl:57-60 already pins that orientation. So the new prose (x=row, y=column; o=0 vertical inter-tile, o=1 horizontal inter-tile) is now consistent with the edge rules, the code, and dnx.

For the record: my earlier #42/#46 reviews flagged the coupler orientation as transposed. That was accurate for the code at that time; it was subsequently fixed in code and tests (before this PR's merge-base), and this PR correctly finishes #47 by documenting the corrected convention. Closes #47 is appropriate - the issue is genuinely resolved, not partially.

Nonblocking

1. The new HFS round-trip note overstates verified behavior for disabled, stale code. The sentence "The legacy HFS/alex1770-style writer emits these coordinate labels unchanged, so round-tripping preserves this row/column, shore-index convention" asserts a round-trip guarantee, but the HFS writer is not loaded (architecture.jl includes only device/topology/chimera; HFS is commented out, tracked in #48) and src/wrapper/hfs/printer.jl does not compile against the current API - it calls a Chimera(model, ...) constructor that no longer exists and reads chimera.linear_terms/chimera.quadratic_terms fields the current Chimera struct does not have. There is also no reader counterpart or test, so "round-tripping" is unverifiable today. A reader of this docstring would reasonably infer HFS round-trip works now. Fix: qualify the claim and point to #48, e.g. "The legacy HFS/alex1770-style writer (currently disabled; see #48) emits the (x, y, o, i) labels verbatim and would preserve this convention," or drop the round-trip sentence until the writer is revived.

2. The dwave-networkx equivalence that motivates this PR is not guarded by a test. test/chimera.jl asserts four specific present/absent edges, which pins the orientation but would not catch a future divergence from dnx across the rest of the graph. Since the docstring and PR description now assert dnx-equivalence, consider adding a direct regression check (the one I ran): build dwave_networkx.chimera_graph(m, n, t; coordinates=true) and assert its edge set equals the wrapper's under identity labels. Cheap, and it locks in the guarantee the docs now make.

Tests run (clean checkout of head f559a01)

• Empirical dnx comparison (above): wrapper graph == dnx coordinate graph under identity labels; 124 edges both.
• julia --project=. test/chimera.jl: 13/13, 12/12, 4/4, 7/7 pass.
• Confirmed at head that the current Chimera exposes only the integer constructor and no linear_terms/quadratic_terms fields, so the HFS printer is stale (basis for nonblocking 1).
• Package loads at head (the comparison script imports and exercises DWave.Chimera). I did not re-run the full Pkg.test() suite for a docstring-only change; the prior PR #46 verification of the same suite remains valid and this change is confined to a @doc block, which test/chimera.jl exercises via import DWave.

Merge-readiness

No blocking issues. The documentation change is correct and improves accuracy, and Closes #47 is justified. The two nonblocking items (qualify the HFS round-trip claim; optionally add a dnx-equivalence test) are worth addressing but do not block. Mergeable on the merits, pending another maintainer's formal verdict.

[bernalde]

Addressed the two review comments in 9ebd3ff.

Main changes:

• Qualified the HFS/alex1770 note to state the writer is currently disabled, references Decide whether to revive or remove the legacy HFS writer #48, and limits the preservation claim to a revived writer.
• Added a Chimera regression test that compares the wrapper edge set with dwave_networkx.chimera_graph(..., coordinates=true) under identity coordinate labels.

Tests:

• git diff --check passed.
• julia --project=. -e "include(\"test/chimera.jl\")" passed.
• julia --project=. -e "using Pkg; Pkg.test()" passed; live D-Wave optimizer tests skipped locally because DWAVE_API_TOKEN is unset.
• GitHub CI passed across all 4 matrix jobs for 9ebd3ff.

Comments intentionally not addressed: none.

Remaining risks or follow-up: HFS writer revival/removal remains tracked separately in #48.