[njzjz-bot]

Summary

• Infer omitted atomic numbers and minimum-distance matrices from element names and the Cordero covalent-radius table
• Pin the exact Li/La default matrix and the {"Li": 2.0} radius-override matrix, including serialized CALYPSO input
• Cover each optional field independently, explicit lists and the shared radius table remaining unchanged after overrides
• Report species-count mismatches and unknown element names clearly; require explicit radii or a matrix where the table has placeholders
• Document the 0.7 radius-sum rule, rounding, overrides and missing-radius behavior

Verification

• CPU exploration suite: 113 tests and 32 subtests passed
• All four review mutants fail: 10x distance factor, neighboring radius index, dropped override loop, constant matrix
• Full Pyright 1.1.318 check with CI-matched NumPy 2.2.6: zero errors
• Synced current master, preserving both merge parents
• Pinned Ruff 0.1.3 formatting, isort 5.12.0 and git diff --check passed

Tests are in a dedicated module to avoid the shared insertion-point conflict with #382.

Closes #357

Agent: dot

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 49 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: ee49cd48-9ba1-4c19-9ca8-2ededf21465e

📥 Commits

Reviewing files that changed from the base of the PR and between f0093fe and 1e5a0ef.

📒 Files selected for processing (3)

• dpgen2/exploration/task/caly_task_group.py
• dpgen2/exploration/task/make_task_group_from_config.py
• tests/exploration/test_caly_defaults.py

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.01%. Comparing base (f0093fe) to head (1e5a0ef).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #383      +/-   ##
==========================================
+ Coverage   85.98%   86.01%   +0.02%     
==========================================
  Files         105      105              
  Lines        6930     6942      +12     
==========================================
+ Hits         5959     5971      +12     
  Misses        971      971              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Retracted. This review was produced without running the mandated /code-review fan-out (the loop skill's section 2); the substitute process used instead has since been shown to miss findings and, in one case, to state a verified-sounding falsehood. Re-reviewing properly.

[wanghan-iapcm]

The fix is right: with the pre-PR source, a flat name_of_atoms that omits either field crashes in make_calypso_input (TypeError: object of type 'NoneType' has no len()), and this closes that. Blocking only on the test, which cannot detect a wrong default.

The test does not pin the distance values

The substance of this change is choosing numeric defaults, but the only assertions on the matrix are shape == (2, 2) and > 0. I mutated the PR's own lines and re-ran the module each time:

* 0.7 -> * 7.0                          (10x wrong minimum distance)   Ran 5 tests ... OK
radius index + 1                        (every element reads its neighbour's radius)   OK
if isinstance(distance_of_ions, dict) -> if False   (dict override loop dropped)   OK
self.distance_of_ions = temp_distance_mtx * 0 + 0.01   (constant matrix)   OK

All four pass. Only a mutant on the atomic-number inference (+1 on line 148) is caught, so the atomic_number == [3, 57] assertion is doing its job; the distance half is not.

For the Li/La config already in the test, the exact matrix through your code path is

np.testing.assert_allclose(
    task_group.distance_of_ions, [[1.79, 2.34], [2.34, 2.90]]
)

which kills the first, second and fourth mutants. The third (the dict-override loop being dropped) needs one more case: nothing anywhere in tests/ pins a computed value for a distance_of_ions dict, and this PR restructures that loop while the description says it preserves per-element overrides. {"Li": 2.0} on the same config gives [[2.8, 2.85], [2.85, 2.9]]; asserting that pins the override path too.

Not blocking

• A mismatched numb_of_species (e.g. 3 with two names) now raises a bare IndexError from the generation loop, where before it reached the assert in caly_input.py that prints the offending values. Such a config was always fatal, so this is message quality only; a numb_of_species != len(self.name_of_atoms) check above the loop would restore it.
• On merge order: this conflicts with #382 only in the test file (both append at the end); the source hunks merge cleanly. #405 merges cleanly with this PR.

[wanghan-iapcm]

These two assertions only establish that a square, positive matrix was produced. I mutated the source four ways (0.7 -> 7.0; radius index +1; dict-override loop replaced by if False; matrix replaced by a constant 0.01) and all five tests stayed green each time.

Please pin the values instead. For this Li/La config:

np.testing.assert_allclose(
    task_group.distance_of_ions, [[1.79, 2.34], [2.34, 2.90]]
)

and one dict case, since no test in the repo pins an override value and this PR restructures that loop: distance_of_ions={"Li": 2.0} on the same config gives [[2.8, 2.85], [2.85, 2.9]].

The atomic_number == [3, 57] line above is fine; it does catch an off-by-one in the symbol table.

[njzjz-bot]

Pinned both exact matrices in 82cd2ab: Li/La defaults [[1.79, 2.34], [2.34, 2.90]] and Li=2.0 override [[2.8, 2.85], [2.85, 2.9]]. Tests also check serialized input, independently omitted fields, explicit lists and that overrides leave the shared defaults intact. All four listed mutants now fail. Added the requested diagnostics/docs, including rejection of placeholder default radii unless explicitly supplied. CPU exploration suite: 69 tests + 7 subtests passed.

Agent: dot