fix: correct dphi index and remove ChartName from testDerivs

[lwJi]

Summary

• Fix dphi[i] to use proper covariant index notation dphi[-i]
• Remove unnecessary ChartName -> cart parameter from PrintEquations call
• Update golden file to reflect output changes (Power() syntax for squared terms)

Test plan

• Verify wolframscript -f test/AllTests.wl passes
• Confirm golden file matches generated output

[claude]

Code Review - PR #27

Summary

This PR makes three related improvements to the testDerivs.wl test file:

1. Corrects tensor index notation from dphi[i] to dphi[-i]
2. Removes unnecessary ChartName -> cart parameter
3. Updates golden file to reflect Power() syntax changes

✅ Positive Findings

1. Correct Tensor Index Convention
The change from dphi[i] to dphi[-i] is mathematically correct and follows proper tensor notation:

• The equation on line 36 is: psi[] = euclid[i, j] dphi[-i] dphi[-j]
• This computes the Euclidean norm: ψ = δⁱʲ ∂ᵢφ ∂ⱼφ
• The metric euclid[i, j] has contravariant indices (upper indices)
• Therefore dphi must have covariant indices (lower indices), denoted as dphi[-i]
• This matches the pattern used throughout the codebase (see test/*/test.wl files where MDD[-i, -j] is consistently used for covariant tensors)

2. ChartName Cleanup
Removing ChartName -> cart is correct:

• Looking at src/Interface.wl:176, PrintEquations has ChartName -> GetDefaultChart[] as the default
• GetDefaultChart[] returns cart for this test (line 20: DefChart[cart, M3, ...])
• Explicitly passing ChartName -> cart is redundant
• This aligns with other test files that use PrintEquations[{Mode -> "Temp"}, TempVarlist] without ChartName (see all files in test/*/test.wl)

3. Golden File Update
The change from dphi1*dphi1 to Power(dphi1,2) is an output format change:

• This appears to be a recent change in how Generato formats squared terms in generated code
• The Power() function is defined in other backends (e.g., test/golden/Nmesh/GHG_rhs.c.golden:7)
• Updating the golden file ensures regression tests pass

🔍 Code Quality Assessment

Architecture & Design: ✅ Good

• Changes align with established conventions in the codebase
• Follows the mathematical rigor expected in numerical relativity code

Consistency: ✅ Excellent

• Tensor index notation now matches other test files
• ChartName removal follows the DRY principle (Don't Repeat Yourself)
• Pattern matches test/CarpetX/test.wl, test/AMReX/test.wl, etc.

Maintainability: ✅ Improved

• Removing redundant parameters reduces maintenance burden
• Correct tensor notation prevents future confusion

⚠️ Recommendations

1. Test Coverage
Before merging, please verify:

wolframscript -f test/AllTests.wl

The PR description mentions this in the test plan but shows it as unchecked. Please confirm all tests pass.

2. Documentation (Minor)
Consider adding a comment in testDerivs.wl explaining the test purpose:

(* Test: Finite difference derivatives with Mode->Derivs *)
(* Computes psi = ||∇φ||² = δⁱʲ ∂ᵢφ ∂ⱼφ using 4th-order accurate stencils *)

This would help future developers understand the mathematical intent.

3. Commit Message (Already Good!)
The commit message clearly explains all three changes. Well done!

🐛 Potential Issues

None identified - This is a clean bug fix with appropriate test updates.

🔒 Security

No security concerns - test file only.

⚡ Performance

No performance impact - test file changes only.

📊 Overall Assessment

Recommendation: ✅ APPROVE (pending test confirmation)

This is a high-quality fix that:

• Corrects a mathematical notation error
• Removes unnecessary code
• Updates tests appropriately
• Follows codebase conventions

The only blocker is confirming the test suite passes. Once that checkbox is marked, this is ready to merge.

---

Review generated by Claude Code 🤖