test(consistent): use curated cases throughout#5606
Merged
Conversation
njzjz
requested review from
iProzd and
wanghan-iapcm
and removed request for
wanghan-iapcm
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAcross the consistent test suites, ChangesMigrate consistent tests to
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
source/tests/consistent/test_type_embedding.py (1)
75-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFail fast on unknown override keys.
A misspelled override is silently ignored here: the merged dict keeps the extra key, but tuple construction only reads
TYPE_EMBEDDING_CASE_FIELDS. That can accidentally leave the baseline value in place and reduce case coverage without any signal.Proposed guard
def type_embedding_case(**overrides: Any) -> tuple: + unknown = set(overrides) - set(TYPE_EMBEDDING_CASE_FIELDS) + if unknown: + raise ValueError(f"Unknown type-embedding case fields: {sorted(unknown)}") case = TYPE_EMBEDDING_BASELINE_CASE | overrides return tuple(case[field] for field in TYPE_EMBEDDING_CASE_FIELDS)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/tests/consistent/test_type_embedding.py` around lines 75 - 77, The type_embedding_case helper silently ignores misspelled override keys when merging TYPE_EMBEDDING_BASELINE_CASE with overrides. Update type_embedding_case to validate overrides against TYPE_EMBEDDING_CASE_FIELDS before building the tuple, and raise an error for any unknown key so bad inputs fail fast instead of falling back to baseline values. Use the existing type_embedding_case function and TYPE_EMBEDDING_CASE_FIELDS/TYPE_EMBEDDING_BASELINE_CASE symbols to keep the check centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/tests/consistent/model/test_dpa1.py`:
- Around line 58-64: The curated DPA1_ENER test cases no longer cover the
smooth-type-embedding branch, so restore that configuration in
DPA1_ENER_CURATED_CASES and keep the related model coverage in test_dpa1.py.
Update the parameterized_cases setup so the suite still exercises the descriptor
mode wired through data(), using the existing DPA1_ENER_CURATED_CASES and
test_dpa1 symbols to place the new case consistently with the current matrix.
In `@source/tests/consistent/model/test_polar.py`:
- Around line 1221-1229: The new curated exclusion cases in the stat test are
reusing the same sampled inputs across dp, pt, and pt_expt, and
compute_or_load_stat mutates natoms in place when atom_exclude_types is
non-empty. Update the test logic in test_polar to apply the same deepcopy
protection already used in test_ener so each backend gets an independent copy of
self.np_sampled and self.pt_sampled before stat computation.
---
Nitpick comments:
In `@source/tests/consistent/test_type_embedding.py`:
- Around line 75-77: The type_embedding_case helper silently ignores misspelled
override keys when merging TYPE_EMBEDDING_BASELINE_CASE with overrides. Update
type_embedding_case to validate overrides against TYPE_EMBEDDING_CASE_FIELDS
before building the tuple, and raise an error for any unknown key so bad inputs
fail fast instead of falling back to baseline values. Use the existing
type_embedding_case function and
TYPE_EMBEDDING_CASE_FIELDS/TYPE_EMBEDDING_BASELINE_CASE symbols to keep the
check centralized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a06fd2ff-36be-4203-a399-ee618de680a4
📒 Files selected for processing (27)
source/tests/consistent/descriptor/test_se_atten_v2.pysource/tests/consistent/descriptor/test_se_r.pysource/tests/consistent/descriptor/test_se_t.pysource/tests/consistent/descriptor/test_se_t_tebd.pysource/tests/consistent/fitting/test_dipole.pysource/tests/consistent/fitting/test_dos.pysource/tests/consistent/fitting/test_dpa4_ener.pysource/tests/consistent/fitting/test_ener.pysource/tests/consistent/fitting/test_polar.pysource/tests/consistent/fitting/test_property.pysource/tests/consistent/loss/test_dos.pysource/tests/consistent/loss/test_ener.pysource/tests/consistent/loss/test_ener_spin.pysource/tests/consistent/loss/test_property.pysource/tests/consistent/loss/test_tensor.pysource/tests/consistent/model/test_dipole.pysource/tests/consistent/model/test_dos.pysource/tests/consistent/model/test_dpa1.pysource/tests/consistent/model/test_ener.pysource/tests/consistent/model/test_frozen.pysource/tests/consistent/model/test_linear_ener.pysource/tests/consistent/model/test_polar.pysource/tests/consistent/model/test_property.pysource/tests/consistent/model/test_zbl_ener.pysource/tests/consistent/test_activation.pysource/tests/consistent/test_learning_rate.pysource/tests/consistent/test_type_embedding.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5606 +/- ##
==========================================
- Coverage 81.98% 81.96% -0.03%
==========================================
Files 959 959
Lines 105430 105430
Branches 4071 4073 +2
==========================================
- Hits 86442 86418 -24
- Misses 17518 17537 +19
- Partials 1470 1475 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
wanghan-iapcm
approved these changes
Jun 30, 2026
iProzd
approved these changes
Jun 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tests
Summary by CodeRabbit