search

lbolla / EMpy

Electromagnetic Python
MIT License
194 stars 83 forks source link

RefractiveIndex(n0_func=...): passes Ones to RIX function #19

Closed demisjohn closed 7 years ago

demisjohn commented 7 years ago

@lbolla : I found a bug in RefractiveIndex.__from_function(), in which the function erroneously passed an array of ones [1,1,1...] via the numpy.ones_like(...) value, instead of the actual wavelength values [1.55e-6,1.54e-6...], to the RIX function supplied by the user.

In my recent fork (to add laser monitor example), I've updated this to return n0_func(wls) * numpy.ones_like(wls) similar to your other functions.
See https://github.com/demisjohn/EMpy/commit/7813056e58b0d4872e176b3593c503f5b4ac9ce2

However, I'm wondering why you multiply the wavelengths wls by numpy.ones_like() in the first place. What is the purpose of this? Should I omit that entirely?

lbolla commented 7 years ago

You must have old code in your branch because that part has already been fixed in https://github.com/lbolla/EMpy/commit/57409d4b0ac601fca8207ed23210ee9b1f1d45c8

Try to pull from origin.

lbolla commented 7 years ago

By the way, the idea behind n0_func is to return and accept an np array, as the docstring says: https://github.com/lbolla/EMpy/blob/master/EMpy/materials.py#L63-L81

demisjohn commented 7 years ago

Hmmm... I had just rebased from this repo before I started the fork - my fork on GitHub says it's only 3 (of my own) commits ahead of yours, I think that means it should be current code, right? That would be unfortunate if I'm working on old code.

The commit you referenced (https://github.com/lbolla/EMpy/commit/57409d4b0ac601fca8207ed23210ee9b1f1d45c8) shows the following erroneous code on line 137 (the last line here):

def __from_function(n0_func, wls):
          wls = numpy.atleast_1d(wls)                 wls = numpy.atleast_1d(wls)
 -        if wls.size == 1:      +        return n0_func(numpy.ones_like(wls))

It passes numpy.ones_like(wls) to the user-supplied function, which means the user's function is always evaluated at wl = 1.0 meters. It should instead pass the actual wavelengths array wls to n0_func() and then multiply the result by ones_like(wls) to ensure it's returning an array. I have fixed this in my fork, here: https://github.com/demisjohn/EMpy/commit/7813056e58b0d4872e176b3593c503f5b4ac9ce2

Thanks for clarifying the purpose of that function, makes perfect sense now. Please let me know if I'm missing something about this error.