Skip to content

Normunpol batch raytrace#225

Open
andibarg wants to merge 36 commits into
MREYE-LUMC:mainfrom
andibarg:normunpol-batch-raytrace
Open

Normunpol batch raytrace#225
andibarg wants to merge 36 commits into
MREYE-LUMC:mainfrom
andibarg:normunpol-batch-raytrace

Conversation

@andibarg

@andibarg andibarg commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Proposed change

1. NormUnpol Batch Raytrace

I added the NormUnpol batch raytrace as a new analysis under raysandspots - hope this is the right place for it. It can be used, for instance, to create spot diagrams (see the official examples).

The batch ray trace is technically not an OpticsStudio analysis but I thought we could perhaps still treat it as such. Note that I could not add the settings to the RayTrace analysis under run_analysis as in self.analysis.Settings.Hx = self.settings.hx because it does not accept lists. Perhaps there is a workaround or better solution to this.

Unfortunately, I cannot run/provide Jupyter notebook examples at the moment. Instead, I prepared this MWE:

import numpy as np
from pathlib import Path
import zospy as zp

zos = zp.ZOS()
oss = zos.connect(mode="standalone")

system_file = Path(zos.Application.SamplesDir) / "Sequential/Objectives/Double Gauss 28 degree field.zmx"
oss.load(system_file)

N = 3000
df = zp.analyses.raysandspots.BatchRayTraceNormUnpol(
    hx=np.linspace(-1,1,N),
    hy=np.zeros(N),
    px=np.zeros(N),
    py=np.zeros(N),
    wavelength=1,
    surface="Image",
).run(oss).data

print(df)

2. Removed warning from SingleRayTrace when surface comment is missing

There is a warning when running the single ray trace in the Double Gauss example. This is simply because some surfaces in the .zmx file do not have a comment. Even though I understand the reason for the warning, I thought it would be nice to avoid the warning in this case.

Type of change

  • Example (a notebook demonstrating how to use ZOSPy for a specific application)
  • Bugfix (non-breaking change which fixes an issue)
  • New analysis (a wrapper around an OpticStudio analysis)
  • New feature (other than an analysis)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests
  • Documentation (improvement of either the docstrings or the documentation website)

Additional information

  • OpticStudio version: 2024 R2.01 Premium

Related issues

Checklist

  • I have followed the contribution guidelines
  • The code has been linted, formatted and tested (see the contribution guidelines).
  • Local tests pass. Please fix any problems before opening a PR. If this is not possible, specify what doesn't work and why you can't fix it.
  • I tested ZOSPy for all supported Python versions (hatch test -a).
  • I added new tests for any features contributed, or updated existing tests.
  • I updated CHANGELOG.md with my changes (except for refactorings and changes in the documentation).

If you updated an example:

  • I executed all examples and verified that they run without errors (hatch run all-examples).

If you contributed an example:

  • I contributed my example as a Jupyter notebook.

@andibarg
andibarg requested review from a team, LucVV, crnh and jwmbeenakker as code owners June 3, 2026 16:20
@andibarg

andibarg commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Btw, sorry if I am not following procedure very well for the pull request. I am not too familiar with it yet.

@crnh crnh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice work, thanks! I agree that zospy.analyses.raysandspots is the best place for this feature, although it is strictly spoken not an analysis.

Btw, sorry if I am not following procedure very well for the pull request. I am not too familiar with it yet.

No problem, we are happy to guide you through the process!

A few general comments:

  • Read the contributing guidelines for instructions on how to lint, format and test ZOSPy. Several points in this review could have been discovered and fixed by using a linter. Furthermore, the linting and formatting checks are required to pass in order to merge the PR.
  • For me, this analysis only works in standalone mode. In extension mode, I get a RemotingException. Is this expected? If so, the analysis should check for the connection mode and raise a clear exception in extension mode.
  • Tests are still missing. The issues with settings validation (where px=[2] is outside the allowed range, but still accepted) could have been caught earlier by adding tests.
  • It would be better to manage the batch ray trace tool with a context manager, to ensure the tool will be closed even after an exception. We can consider providing a utility function for this in a separate PR.

I agree the warning about the comment in the raytrace analysis is annoying; I always suppress it in my code, but thanks for addressing this. Since this is a separate change, it belongs in a separate PR. This time we'll accept this, but next time please open separate PRs for different changes.

Comment thread zospy/analyses/raysandspots/__init__.py Outdated
Comment thread zospy/analyses/raysandspots/batch_ray_trace.py Outdated
Comment thread zospy/analyses/raysandspots/batch_ray_trace.py Outdated
Comment thread zospy/analyses/raysandspots/batch_ray_trace.py Outdated
Comment thread zospy/analyses/raysandspots/batch_ray_trace.py Outdated
Comment thread zospy/analyses/raysandspots/batch_ray_trace.py Outdated
Comment thread zospy/analyses/raysandspots/batch_ray_trace.py Outdated
Comment thread zospy/analyses/raysandspots/batch_ray_trace.py Outdated
Comment thread zospy/analyses/raysandspots/batch_ray_trace.py Outdated
Comment thread zospy/tools/batch_raytrace.py Outdated
@andibarg

andibarg commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks a lot for your review!

I tried implementing your points as well as I could.

Just a comment about a change: I am not very happy with using Sequence instead of list because it bans numpy arrays (see my MWE above). But I cannot find a better solution at the moment.

Btw, I think it is expected that the batch raytrace does not work in extension mode because it is not really an analysis that can be run by itself from within OpticStudio.

@crnh

crnh commented Jun 5, 2026

Copy link
Copy Markdown
Member

Just a comment about a change: I am not very happy with using Sequence instead of list because it bans numpy arrays (see my MWE above). But I cannot find a better solution at the moment.

Yes, I noticed that as well. The best way to allow NumPy arrays as well is probably typing these attributes with a union of Sequence and NDArray. This requires a custom validator to ensure additional constraints are applied to the array items as well.

@crnh crnh mentioned this pull request Jun 5, 2026
15 tasks
@Wellidontcare

Copy link
Copy Markdown

Something that could be cool to implement in the future (not a feature request):

Zemax batch tracing, needs you to add the rays one by one, this is a huge slowdown due to python loops and API overhead, retrieval of traced rays suffers from this as well.

There is the option to outsource the ray adding and retreival to C#: https://optics.ansys.com/hc/en-us/articles/42661765866899-Batch-Processing-of-Ray-Trace-Data-using-ZOS-API-in-MATLAB-or-Python (the example is bugged by the way, but the idea is there)

With this extra wrapper you could pass and retrieve the rays via numpy arrays. If this would be implemented in ZOSPy, the project would need to ship the C# DLL though (question is, where to put it, as git does not like binaries), cross platform should not be an issue, as Zemax is Windows only anyways.

@andibarg

andibarg commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

I managed to transfer the batch raytrace over to zospy.tools and to me this looks good now. Please let me know what you think.

Regarding unit tests, I am having difficulties running hatch correctly. I think this might be for the same reason why I cannot run/provide Jupyter notebook examples (long story). I wanted to update the Double Gauss example with my MWE but I am afraid I am a bit stuck at the moment.

@Wellidontcare: Sounds interesting. For my use cases, the python loop does not really cause a huge slowdown but I can imagine there are applications where this matters. Would be cool to implement in the future.

@crnh crnh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating this! Due to a conference, the final tests before merging this PR will probably happen next week.

This PR should be merged after #226, because it depends on features that are introduced there.

Comment thread zospy/tools/batch_raytrace.py Outdated
Comment thread zospy/tools/batch_raytrace.py Outdated
Comment thread zospy/tools/batch_raytrace.py Outdated
Comment thread zospy/tools/batch_raytrace.py
@crnh

crnh commented Jun 8, 2026

Copy link
Copy Markdown
Member

Regarding unit tests, I am having difficulties running hatch correctly. I think this might be for the same reason why I cannot run/provide Jupyter notebook examples (long story).

Can you let us know what goes wrong, so we can maybe help you and if applicable update our contribution guidelines? If you don't want to discuss this as part of this PR, please drop us a message at zospy@mreye.nl.

If the issue is related to hatch, you can always run the tests with pytest. hatch primarily makes setting up the test environment easier.

@crnh

crnh commented Jun 16, 2026

Copy link
Copy Markdown
Member

@andibarg I just merged #226. Please complete these steps to finalize this PR:

  • Rebase this branch onto main
  • Address the open review comments
  • Add unit tests for the batch ray trace tool

@crnh

crnh commented Jun 26, 2026

Copy link
Copy Markdown
Member

@andibarg I added unit tests for the batch ray trace tool and updated the changelog. Once the tests pass for all supported Python versions, this one is good to go!

@crnh
crnh self-requested a review June 26, 2026 11:36
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