Check CGAL wall intersection at microstep intervals

[krystophny]

User description

Summary

• Add macrostep_with_wall_check() subroutine that checks STL wall intersection at intervals within macrosteps (every 32 microsteps or at macrostep end for small macrosteps)

Depends on: #320 (CGAL 6.0.1 update)

Motivation

With logarithmic macrostep time grid, late macrosteps become very large. The previous implementation checked wall intersection only once per macrostep, which could potentially miss intersections during large steps.

The new implementation checks at regular intervals within macrosteps to ensure accurate wall hit detection regardless of macrostep size.

Test plan

• 1ms CGAL run: Results identical to old code (both show 0 particles in 30-60° zeta bin)
• 10ms CGAL run: Results nearly identical to old code
• Build succeeds

Notes

Testing showed that for prompt losses (<1ms), the microstep checking gives identical results to macrostep-only checking. This confirms the original implementation was correct for early losses. The microstep checking adds ~2x overhead but provides additional safety for long runs with large macrosteps.

---

PR Type

Enhancement, Bug fix

---

Description

• Add macrostep_with_wall_check() for periodic wall intersection checks

  • Checks STL wall at intervals within macrosteps (every 32 microsteps)
  • Prevents missing intersections during large late-stage macrosteps

• Refactor trace_orbit() to use wall-aware macrostep routine

  • Simplifies variable declarations and control flow
  • Properly handles early particle exit with NaN padding

• Update CGAL dependency from 5.6.1 to 6.0.1

  • Fixes GCC 15 compatibility issues in boost/graph/iterator.h

---

Diagram Walkthrough

flowchart LR
  A["trace_orbit loop"] --> B{"wall_enabled?"}
  B -->|Yes| C["macrostep_with_wall_check"]
  B -->|No| D["macrostep"]
  C --> E["Check wall every 32 microsteps"]
  E --> F["Detect intersection early"]
  D --> G["Single macrostep check"]
  F --> H["Exit with ierr_orbit=77"]
  G --> H

Loading

File Walkthrough

	Relevant files
Enhancement	simple_main.f90 Implement microstep-level wall intersection checking          src/simple_main.f90 Extracted macrostep() subroutine from inline code for reusability Added new macrostep_with_wall_check() subroutine with periodic wall checking logic Refactored trace_orbit() to conditionally call wall-aware or standard macrostep Removed unused local variables (u_ref_cur, x_cur, x_hit, etc.) from trace_orbit() Improved trajectory storage and NaN padding for early particle exits +108/-64
Dependencies	CMakeLists.txt Upgrade CGAL to version 6.0.1                                                        CMakeLists.txt Updated CGAL version from 5.6.1 to 6.0.1 in FetchContent URL Updated version references in status messages and comments +3/-3

---

[qodo-code-review]

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Unpinned dependency fetch Description: Fetching CGAL via FetchContent_Declare(URL https://...) without an integrity pin (e.g., URL_HASH) creates a supply-chain risk where a compromised upstream release, MITM, or cache poisoning could introduce malicious headers into the build. CMakeLists.txt [209-223] Referred Code include(FetchContent) FetchContent_Declare( cgal URL https://github.com/CGAL/cgal/releases/download/v6.0.1/CGAL-6.0.1.tar.xz DOWNLOAD_EXTRACT_TIMESTAMP TRUE ) FetchContent_GetProperties(cgal) if(NOT cgal_POPULATED) FetchContent_Populate(cgal) endif() # CGAL 6.x is header-only, just add include path set(CGAL_INCLUDE_DIR "${cgal_SOURCE_DIR}/include") message(STATUS "CGAL enabled via FetchContent: 6.0.1 (header-only)") message(STATUS "CGAL include dir: ${CGAL_INCLUDE_DIR}") add_compile_definitions(SIMPLE_ENABLE_CGAL)
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Unused variables: New variables u_ref_prev and x_prev are introduced in trace_orbit but are not used in the new implementation, reducing code clarity and self-documentation. Referred Code real(dp) :: u_ref_prev(3) real(dp) :: x_prev(3), x_prev_m(3) integer :: it, ierr_orbit, it_final Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No audit logging: The new wall-intersection early-exit path sets ierr_orbit=77 without any accompanying audit-style logging, so it is unclear whether this action should be recorded for traceability. Referred Code subroutine macrostep_with_wall_check(anorb, z, kt, ierr_orbit, ntau_local, & ipart, x_prev_m) use alpha_lifetime_sub, only: orbit_timestep_axis use orbit_symplectic, only: orbit_timestep_sympl type(tracer_t), intent(inout) :: anorb real(dp), intent(inout) :: z(5) integer(8), intent(inout) :: kt integer, intent(out) :: ierr_orbit integer, intent(in) :: ntau_local integer, intent(in) :: ipart real(dp), intent(inout) :: x_prev_m(3) integer :: ktau, wall_check_interval real(dp) :: u_ref_cur(3), x_cur(3), x_cur_m(3) real(dp) :: x_hit_m(3), x_hit(3), normal_m(3) real(dp) :: vhat(3), vnorm, cos_inc real(dp) :: u_ref_hit(3) logical :: hit integer :: ierr_from_cart ... (clipped 63 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Missing failure context: When ref_coords%from_cart fails (ierr_from_cart != 0), the code still flags a wall-hit (ierr_orbit = 77) and exits without preserving or reporting the conversion failure context, which may hinder debugging and correct classification of the failure. Referred Code ierr_from_cart = 1 end select if (ierr_from_cart == 0) then call ref_to_integ(u_ref_hit, z(1:3)) end if ierr_orbit = 77 exit end if Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Unstructured status logs: New build-time message(STATUS ...) output is unstructured and it is unclear whether project policy requires structured logging (though it does not appear to include sensitive data). Referred Code # CGAL 6.x is header-only, just add include path set(CGAL_INCLUDE_DIR "${cgal_SOURCE_DIR}/include") message(STATUS "CGAL enabled via FetchContent: 6.0.1 (header-only)") message(STATUS "CGAL include dir: ${CGAL_INCLUDE_DIR}") Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix bug skipping wall check In macrostep_with_wall_check, remove the if (ktau > 1) condition to fix a bug that skips the wall intersection check on the first microstep of a macrostep. This ensures the check is performed correctly, preventing missed wall collisions. src/simple_main.f90 [653-736] subroutine macrostep_with_wall_check(anorb, z, kt, ierr_orbit, ntau_local, & ipart, x_prev_m) ... integer :: ktau, wall_check_interval ... ! Check wall every N microsteps to limit overhead ! For small macrosteps (ntau_local<=32), check at end only ! For larger macrosteps, check every 32 microsteps wall_check_interval = max(1, min(32, ntau_local)) do ktau = 1, ntau_local ... ! Check wall intersection periodically or at end of macrostep if (mod(ktau, wall_check_interval) == 0 .or. ktau == ntau_local) then call integ_to_ref(z(1:3), u_ref_cur) call ref_coords%evaluate_cart(u_ref_cur, x_cur) x_cur_m = x_cur*chartmap_cart_scale_to_m - if (ktau > 1) then - call wall_intersect_check(x_prev_m, x_cur_m, hit, x_hit_m, normal_m) + call wall_intersect_check(x_prev_m, x_cur_m, hit, x_hit_m, normal_m) ... x_prev_m = x_cur_m end if ... end do end subroutine macrostep_with_wall_check [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical bug in the new macrostep_with_wall_check subroutine where the if (ktau > 1) condition causes the wall intersection check to be skipped for the first segment of a macrostep, potentially missing a wall collision.	High
	Seed initial wall‐check position In the trace_orbit subroutine, initialize the x_prev_m variable before the main timestep loop. This ensures the wall-checking logic has a well-defined starting position for the first segment. src/simple_main.f90 [541-542] real(dp) :: u_ref_prev(3) real(dp) :: x_prev(3), x_prev_m(3) +! initialize previous microstep position before wall checks +call integ_to_ref(z(1:3), u_ref_prev) +call ref_coords%evaluate_cart(u_ref_prev, x_prev) +x_prev_m = x_prev * chartmap_cart_scale_to_m [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that the local variable x_prev_m in trace_orbit is used uninitialized in the first call to macrostep_with_wall_check, which is a critical bug that would lead to incorrect wall collision checks.	High
	Initialize passing variable Initialize the logical variable passing before it is used in the call to increase_confined_count. Currently, it is uninitialized, which could lead to incorrect behavior. src/simple_main.f90 [608] +! determine if particle is currently confined +passing = .true. call increase_confined_count(it, passing) Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that the passing variable is used in increase_confined_count without being initialized within the trace_orbit subroutine, which is a significant bug.	Medium
High-level	Refactor to eliminate code duplication The macrostep_with_wall_check and macrostep subroutines contain a duplicated integration loop. To improve maintainability, merge them into a single subroutine that performs the wall check conditionally. Examples: src/simple_main.f90 [627-736] subroutine macrostep(anorb, z, kt, ierr_orbit, ntau_local) use alpha_lifetime_sub, only: orbit_timestep_axis use orbit_symplectic, only: orbit_timestep_sympl type(tracer_t), intent(inout) :: anorb real(dp), intent(inout) :: z(5) integer(8), intent(inout) :: kt integer, intent(out) :: ierr_orbit integer, intent(in) :: ntau_local ... (clipped 100 lines) Solution Walkthrough: Before: subroutine macrostep(...) do ktau = 1, ntau_local // ~10 lines of integration logic end do end subroutine subroutine macrostep_with_wall_check(...) do ktau = 1, ntau_local // ~10 lines of duplicated integration logic if (time_for_wall_check) then // wall check logic end if end do end subroutine // in trace_orbit if (wall_enabled) then call macrostep_with_wall_check(...) else call macrostep(...) end if After: subroutine macrostep(..., check_wall) do ktau = 1, ntau_local // ~10 lines of integration logic if (check_wall .and. time_for_wall_check) then // wall check logic end if end do end subroutine // in trace_orbit call macrostep(..., check_wall=wall_enabled) Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies significant code duplication between the new macrostep_with_wall_check and macrostep subroutines, which negatively impacts future maintainability.	Medium
More