Skip to content

Conversation

@paulromano
Copy link
Contributor

Description

This PR adds a generic region-based container for openmc.model.pack_spheres, enabling packing in arbitrary closed Region objects with a required finite bounding box and explicit volume. pack_spheres now accepts bounding_box/volume kwargs and uses a fixed container selection order to preserve existing behavior. The new _RegionContainer supports analytic boundary clearance for simple shapes and uses conservative directional checks otherwise:

  • Analytic fast path: If the region is an Intersection of Halfspace objects with planes, spheres, cylinders, or cones, we compute exact clearance to each surface and require it to be at least the sphere radius. This gives a precise, fast boundary test.

  • Generic fallback: Otherwise we do a conservative directional probe. We generate ~50 unit vectors on the sphere using a Fibonacci lattice (low-discrepancy, evenly spread points). A candidate center is accepted only if every probe point at center + safety * radius * direction is inside the region. The 50 points are a practical balance between cost and coverage; the Fibonacci pattern avoids clustering at the poles, so you get better angular uniformity than random or latitude/longitude grids. A small safety factor (1.001) reduces false positives from numerical edge cases near the boundary.

This approach is intentionally conservative: it may reject some valid centers but should not accept invalid ones, and it keeps the cost predictable for arbitrary CSG regions.

@ZoeRichter @MendesMichael if either of you want to try this out on your use cases, I would love know how it goes and if you encounter any problems. In particular, I'm curious about the performance of this versus your specialized implementation from #3739.

Marking as draft since this was 99% coded by GPT 5.2 Codex and I haven't really reviewed/optimized to my liking, but I wanted to make it available for others to start playing around with.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 15) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@MendesMichael
Copy link

@paulromano I'll try this out and let you know how it goes! Thank you.

@ZoeRichter
Copy link

I still have the little sample cone I made before - I'll run both versions and report back on the time.

@ZoeRichter
Copy link

ZoeRichter commented Jan 30, 2026

image

Something goes very wrong around 0.3 pf. I'm checking 40% pf now, but I wonder if the issue is that the limit for switching from RSP to CRP needs to be lower to use this method.

edit: added line from specific-region 0.4 of results. I also realized I made the bounding box too big (it encompassed the cylinder as well) so I made the bounding box tighter. I re-did 0.1 and 0.2 with the smaller bounding box, and 0.3 is running now. I'll update this comment with the performance as I test.

@paulromano paulromano marked this pull request as draft January 30, 2026 22:25
@ZoeRichter
Copy link

Here is the input file I'm running. I'm only updating the packing fraction between runs, and the only difference between the generic and specific runs is whether or not I supply the volume/bounding box (and the branch my openmc install is on)

jt-core-test.py

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