Repository navigation
Hexagonal distribution traces significantly more rays than described in optic.trace() #757
Description
Describe the bug
'hexapolar' distribution is a function of the number of rings rather than the number of points/rays. optic.trace() is a function of num_rays, so when you pass num_rays = 100 the hexapolar distribution actually generates a distribution of 20201 rays which are then traced through the optics. All other distributions are a function of num_points so this is not an issue for them, but it is important since 'hexapolar' is the default distribution for the ray tracing engine.
To Reproduce
Run optic.trace(....,num_rays=100, distribution="hexapolar")
output will have a dimension equal to 20201 instead of 100.
Expected behavior
Should be modified to support num_rays rather than rings(
Screenshots
Copy of the latest code in Optiland:
class HexagonalDistribution(BaseDistribution):
"""A class representing a hexagonal distribution.
Generates points in a hexagonal pattern. The total number of points is
`1 + 3 * num_rings * (num_rings + 1)`, including the center point.
Attributes:
x: Array of x-coordinates of the generated points.
y: Array of y-coordinates of the generated points.
"""
def generate_points(self, num_rings: int = 6):
"""Generate points in a hexagonal distribution.
Args:
num_rings: Number of rings in the hexagonal distribution.
Defaults to 6.
"""
x = be.zeros([1])
y = be.zeros([1])
r = be.linspace(0, 1, num_rings + 1)
for i in range(num_rings):
num_theta = 6 * (i + 1)
theta = be.linspace(0, 2 * be.pi, num_theta + 1)[:-1]
x = be.concatenate([x, r[i + 1] * be.cos(theta)])
y = be.concatenate([y, r[i + 1] * be.sin(theta)])
self.x = x
self.y = y
Environment
All environments
Additional context
N/A
Activity
RayPragma commentedon Sep 9, 2026
We came at this from the other side: we've been driving hexapolar traces through
num_raysfor a while, and after getting bitten once we ended up writing the ringsemantics down as a documented invariant rather than treating it as a bug. Two things from
that experience that might be useful here.
The counts. Confirming on
CookeTriplet, current master:i.e.
1 + 3·N·(N+1). (Fornum_rays=100that gives 30301, not the 20201 in the report —worth double-checking which surface/array the 20201 was read off, since it may be a
different count.)
On the proposed fix. I'd gently argue against making
num_raysmean a point count forhexapolar, for a reason that isn't obvious until you optimize with it: the ring count is
the parameter you actually want to control. Hexapolar's value is that it is a structured,
rotationally symmetric sampling — an arbitrary point count can't be honoured without either
rounding to the nearest ring count anyway, or breaking the structure that makes the
distribution worth using. And the ring count is what maps onto cost in a predictable way.
We settled on three different ring counts for three purposes:
absolute RMS. Going from 24 rings to 11 was a ~5× speedup in our optimization loop with
no change in where it converged, and one LM Jacobian is (n_vars+1) × n_operands ×
n_fields traces, so this is the number that dominates runtime.
it's evaluated once.
If
num_rayssilently became a point count, all three of those would change meaning, andevery existing script would quietly trace a different number of rays and get slightly
different RMS values with no error and no diff. That seems worse than the current
surprise.
What does seem worth fixing is that the same parameter name means two different things
depending on the value of a string argument — points for every other distribution, rings
for hexapolar — and nothing in the signature says so. Options that keep results stable:
num_rings=explicitly for hexapolar and keepnum_rays=working as an alias,documenting the two as equivalent-but-differently-named.
1 + 3·N·(N+1)at every entry point (optic.trace,the operand
input_data,SpotDiagram), which today only says "num_rays".30301 can find that out without reading the distribution class.
Happy to open a docs PR for (2) if that's the direction you'd want — it's the smallest
change that removes the trap without altering anyone's numbers.
HarrisonKramer commentedon Sep 16, 2026
Hi @kumar-vishesh and @RayPragma,
Thanks for the discussion on this. I agree with @RayPragma's assessment: preserving the ring-based semantics for
hexapolaris critical so we don't alter sampling structure, runtime costs, or convergence in existing optimization workflows and scripts. We'd also prefer not to introduce an extranum_ringsparameter at this time.That said, clarifying the documentation at the entry points (e.g. in the$1 + 3N(N+1)$ ring formula would be very helpful to prevent this surprise.
optic.trace()parameter description and therms_spot_sizeoperand docs) to explicitly note the@RayPragma, we'd gladly welcome a docs PR for option (2) if you're still open to opening one! I'll assign you here.
RayPragma commentedon Oct 22, 2026
Hi @HarrisonKramer, apologies for the long silence on this. You assigned it to me over a month
ago and I let it slip; sorry for leaving the issue hanging after offering the PR. It's up now: #988
It only touches docs and docstrings, so no results change:
Optic.trace()andRealRayTracer.trace(): forhexapolar,num_raysis the ring count N, giving1 + 3·N·(N+1)rays per field point (the default of 100 traces 30301). The Notes now say whatnum_raysmeans for each built-in distribution. They previously called theuniformvalue"total number of rays per axis", which was also misleading.
rms_spot_sizeoperand,SequencedOptic.trace(),RmsWavefrontErrorVsFieldandoptimal_focus_distance()get the same one-line note.rms_spot_sizeexample withnum_rays=100traces 30301 rays per evaluation.@kumar-vishesh, thanks again for the report, and sorry it took this long to close out.
docs: document the hexapolar ring count behind num_rays (#757) (#988)