Skip to content

refractive index computed for a given temperature #203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jul 12, 2025

Conversation

mattemilio
Copy link
Contributor

In case the thermal dispersion coefficients are available in the yml file the coefficients are read and used to compute refractive index of the material for a specific temperature
This is important for optical designs that do not work at ambient temperature (e.g. some optical systems in space missions)
I am not a programmer and this is the first time I contribute to a repository. Please forgive any potential mistake

Updated to compute refractive index also as a function of temperature
@HarrisonKramer
Copy link
Owner

Hi @mattemilio,

Thanks for the contribution! Adding support for temperature- and pressure-dependent refractive index is a great improvement, and this will be very useful for many applications.

Since this is your first time contributing: welcome! Don’t worry about formatting or coding conventions. What matters most is that you’ve taken the time to improve the package, and that’s always appreciated.

Before merging, there are just a few things I’d like to clean up:

  • The docstrings should follow Google style to stay consistent with the rest of the project. Tools like Gemini or GPT can help reformat them automatically if you like.
  • There are a few small typos and minor issues in the comments and docstrings that need correction.
  • Some of the constants used in _nair() would benefit from brief comments or references, just so it’s clear where they come from. We try to avoid using "magic numbers" without explanation.

I’m happy to make these changes myself, but if you’d prefer to continue working on the PR, that’s very welcome too. If you push new commits to this branch, they’ll appear automatically here. I can also push my own edits to your branch if you’re okay with that.

Let me know how you’d like to proceed, and thanks again for your contribution!

Best,
Kramer

@mattemilio
Copy link
Contributor Author

Thanks @HarrisonKramer for the feedback.
I am happy to contribute.
I am just learning how to work on github and I am happy if you do the changes yourself.
Concerning the formulas I used the approach in https://support.zemax.com/hc/en-us/articles/1500005576002-How-OpticStudio-calculates-refractive-index-at-arbitrary-temperatures-and-pressures

Kind regards
Matteo

mattemilio and others added 3 commits June 21, 2025 21:58
fixed 1 bug
sometimes the reference temperature exist in the yaml file but not the thermal dispersion coefficients (e.g. fused_silica). In that case there was a problem in the previous version
Copy link

codecov bot commented Jun 30, 2025

Codecov Report

Attention: Patch coverage is 97.50000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
optiland/materials/material_file.py 97.50% 1 Missing ⚠️

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #203   +/-   ##
=======================================
  Coverage   95.51%   95.51%           
=======================================
  Files         163      163           
  Lines        9133     9169   +36     
=======================================
+ Hits         8723     8758   +35     
- Misses        410      411    +1     
Files with missing lines Coverage Δ
optiland/materials/material_file.py 97.94% <97.50%> (-0.17%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@HarrisonKramer
Copy link
Owner

Thanks again for writing this, @mattemilio!

I made some formatting changes, keeping the original mathematics from your code. We still need to add tests for the new functionality before we can merge. That could involve making comparisons against commercial code or just systematically going through the equations by hand for a few materials and comparing them to the values calculated in Optiland.

I will try to do these comparisons and add some tests in the coming days. You can also do this if you prefer - just let me know if so.

Regards,
Kramer

@mattemilio
Copy link
Contributor Author

Thanks a lot @HarrisonKramer.
I can work on the comparison with commercial software.
Kind regards
Matteo

A correction need to be applied for the input wavelength to compute the base_relative_n in case of arbitrary temperature and pressure
@mattemilio
Copy link
Contributor Author

I obtain very similar refractive index values but not exactly the same.
For instance refractive index N-BK7 at -40 degrees and 0 atm is 1.51888076 with optiland and with Zemax I get 1.5188807836

I simulated an aspheric N-BK7 lens in Zemax optimised to have perfect geometrical spot on axis at a temperature of -40 degrees and P = 0 atm. I compare the spot size between optiland and Zemax
spots_zemax_optiland.pdf
spot_optiland_20deg1atm
singlelens
spot_optiland_minus40deg0atm
spot_optiland_minus40deg1atm

@HarrisonKramer
Copy link
Owner

Hi @mattemilio,

Thanks for making the comparison. The results look very close, and the discrepancies seem minor enough to merge for now, especially given potential differences in coefficients or even numerical precision.

Before merging, I'd like to add unit tests to structurally test the model outputs against the underlying equations. I will do this over the coming days and will push my changes to this PR.

Regards,
Kramer

@HarrisonKramer
Copy link
Owner

Hi @mattemilio,

I just added a few more tests to improve coverage. Everything else remains as it was. I also added you to the authors page. If you prefer not to be listed, just let me know.

Thanks again for the solid addition. For a first contribution, it's excellent - and we'd be happy to see more ideas, suggestions or PRs from you in the future!

Best,
Kramer

@HarrisonKramer HarrisonKramer merged commit 7474f9c into HarrisonKramer:master Jul 12, 2025
12 checks passed
@HarrisonKramer
Copy link
Owner

Hi @mattemilio,

We have a discord server for all contributors. We use it to discuss new features, get feedback on implementations, etc. Feel free to join if you like, even just as an observer.

Link to join:
https://discord.gg/ejNgkP9t

Regards,
Kramer

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.

2 participants