Extension dispersion relation to include pressure and temperature dependence

[deimemiche]

No description provided.

[nzhagen]

I just added the temperature dependence code into the repo, with substantial modifications.

There was one place that I would particularly like to know about -- the code where int(nm[6]) and so on was replaced with int(float(nm[6])). What behavior was driving that change? Perhaps a difference in Python versions?

[deimemiche]

Hi,
I just compared the temperature dependence code and have a few questions:

• Why did you remove the pressure dependence of the refractive index? According to the reference from SCHOTT TIE-19, all "refractive index values given in the optical glass catalog apply for an air pressure of
  0.10133·10^6 Pa". The equations I used modify the refractive index of every glass to account for operations under e.g. vaccum conditions. Your modifications removed this feature, again.

• Why did you not use my method to calculate the refractive index with numpy array operations (e.g. the einsum operand). In the plot command for the temperature dependence, you rely now on a list operation, which is much slower compared to the numpy operations.

• The code int(float(...)) was probably due to a difference in python version. It works now for me without it.

Greetings, Michael

[nzhagen]

Michael, Sorry for the absurdly slow reply. And my apologies about removing the functionality -- I basically misunderstood the purpose of the pressure dependence code. I'll try to add that back in. Since there were no comments that I could go on to tell me what the changes were intended to effect, I had to guess, and I got that wrong this time. Sorry about that. For the "einsum" operation, it is typically my preference to lean towards easy-to-understand code rather than optimize for speed except where necessary. I could guess that you were trying to optimize the code for speed, but I've never needed the extra speed boost myself. So I prefer to keep the operation as transparent as possible. Please let me know if the list-based method works too slowly for you. Thanks again for the pull request -- regardless of the differences in coding style I'm glad to have the feedback and improvements! - Nathan

…

On Tue, Feb 7, 2017 at 5:34 AM, deimemiche ***@***.***> wrote: Hi, I just compared the temperature dependence code and have a few questions: - Why did you remove the pressure dependence of the refractive index? According to the reference from SCHOTT TIE-19, all "refractive index values given in the optical glass catalog apply for an air pressure of 0.10133·10^6 Pa". The equations I used modify the refractive index of every glass to account for operations under e.g. vaccum conditions. Your modifications removed this feature, again. - Why did you not use my method to calculate the refractive index with numpy array operations (e.g. the einsum operand). In the plot command for the temperature dependence, you rely now on a list operation, which is much slower compared to the numpy operations. - The code int(float(...)) was probably due to a difference in python version. It works now for me without it. Greetings, Michael — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADVSsV1VUoP80UyZu_AX4Stv8oBfpPhVks5rZ4PIgaJpZM4KNy5s> .

[nzhagen]

Michael,

I've been trying to merge your update, but your update contains hundreds of lines of stylistic differences with the repo version, and so I can only update the part about the "force_update" keyword. Can you tell me what the function of that is? We should put a line into the code describing what it is for.

[nzhagen]

Michael, I believe the pressure-dependence functionality is in place within the code. Do you have a good method for testing it to verify correctness?

[deimemiche]

Dear Nathan, I am currently finishing my PhD thesis, so I am quite busy and my answer are therefore a bit slow. I will also answer shortly to your first question and then put some time into zemaxglass. To your question: I verified it by comparing it with values directly calculated from ZEMAX. I could create data with ZEMAX which we could use in unit testing for comparison. Greetings Michael Von: nh [mailto:[email protected]] Gesendet: Samstag, 1. Juli 2017 07:06 An: nzhagen/zemaxglass <[email protected]> Cc: deimemiche <[email protected]>; Author <[email protected]> Betreff: Re: [nzhagen/zemaxglass] Extension dispersion relation to include pressure and temperature dependence (#2) Michael, I believe the pressure-dependence functionality is in place within the code. Do you have a good method for testing it to verify correctness? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2 (comment)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AQ3e9sTIqxUtMgmYYgq2qX-VohdqQ8BUks5sJdO2gaJpZM4KNy5s> .

[nzhagen]

Michael - Sounds good. I know from personal experience how hard it can be to devote time to this stuff when there are pressures on your time from everywhere else. Thanks for the help, and I can definitely wait until a time that works for you.