Skip to content

fixed forbidden samrai Index to IntVector conversion#1113

Merged
nicolasaunai merged 3 commits intoPHAREHUB:masterfrom
UCaromel:fix-samrai-intvector
Dec 4, 2025
Merged

fixed forbidden samrai Index to IntVector conversion#1113
nicolasaunai merged 3 commits intoPHAREHUB:masterfrom
UCaromel:fix-samrai-intvector

Conversation

@UCaromel
Copy link
Contributor

@UCaromel UCaromel commented Dec 2, 2025

the title says it all

@coderabbitai
Copy link

coderabbitai bot commented Dec 2, 2025

📝 Walkthrough

Walkthrough

Replaced explicit SAMRAI::hier::IntVector initializations with brace-initialized IntVector in tests and switched local variable declarations to auto const in FieldGeometry::toFieldBox. No behavioral or public API changes.

Changes

Cohort / File(s) Change Summary
Type deduction in core header
src/amr/data/field/field_geometry.hpp
Replaced explicit SAMRAI::hier::IntVector local variable declarations (lower, upper) with auto const to use type deduction; no logic change.
Explicit IntVector construction in tests
tests/amr/data/particles/copy_overlap/test_particledata_copy_periodicNd.cpp, tests/amr/data/particles/stream_pack/test_main.cpp
Changed initializers of SAMRAI::hier::Transformation to pass an explicitly constructed SAMRAI::hier::IntVector{...} (wrapping the existing subtraction expressions) instead of relying on implicit construction; no behavioral change.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify the auto const deductions in field_geometry.hpp yield the intended IntVector type and no surprising references/temporaries are introduced.
  • Check the test initializers compile without ambiguity and still match expected semantics in Transformation constructors.

Possibly related PRs

Suggested reviewers

  • PhilipDeegan

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: fixing a forbidden SAMRAI Index to IntVector conversion issue.
Description check ✅ Passed The description references the title and is related to the changeset, though it is minimal and relies on the title for clarity.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7caa514 and b7ce5c2.

📒 Files selected for processing (2)
  • tests/amr/data/particles/copy_overlap/test_particledata_copy_periodicNd.cpp (1 hunks)
  • tests/amr/data/particles/stream_pack/test_main.cpp (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Analyze (cpp)
  • GitHub Check: Analyze (python)
  • GitHub Check: build (ubuntu-latest, clang)
  • GitHub Check: build (macos-latest)
  • GitHub Check: build (ubuntu-latest, gcc)
  • GitHub Check: build (macos-14)
🔇 Additional comments (2)
tests/amr/data/particles/stream_pack/test_main.cpp (1)

68-69: LGTM! Correct fix for SAMRAI type mismatch.

The explicit IntVector construction resolves the compilation error where Index::operator- returns an Index but Transformation expects an IntVector. This aligns with the SAMRAI API requirements.

tests/amr/data/particles/copy_overlap/test_particledata_copy_periodicNd.cpp (1)

60-61: LGTM! Resolves the reported compilation error.

The explicit IntVector wrapper directly fixes the compilation error reported in the PR objectives at this exact location. The change converts the Index arithmetic result to the IntVector type expected by Transformation.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

@PhilipDeegan
Copy link
Member

there's also this

tests/amr/data/particles/copy_overlap/test_particledata_copy_periodicNd.cpp:60:34: error: could not convert ‘{SAMRAI::hier::Index::operator-(const SAMRAI::hier::Index&) const((* &((twoParticlesDataNDTouchingPeriodicBorders<PHARE::core::DimConst<3> >*)this)->twoParticlesDataNDTouchingPeriodicBorders<PHARE::core::DimConst<3> >::sourceDomain.SAMRAI::hier::Box::upper()))}’ from ‘<brace-enclosed initializer list>’ to ‘SAMRAI::hier::Transformation’
   60 |     SAMRAI::hier::Transformation transformation{destPdat.getGhostBox().lower()

@PhilipDeegan PhilipDeegan self-requested a review December 2, 2025 22:19
@nicolasaunai nicolasaunai merged commit c18a8e5 into PHAREHUB:master Dec 4, 2025
12 checks passed
UCaromel added a commit to UCaromel/PHARE that referenced this pull request Feb 6, 2026
* fixed forbidden samrai Index to IntVector conversion

* update tests

---------

Co-authored-by: deegan <philip.deegan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants