faitdivers / pyao

PyAO - Adaptive Optics simulation package
6 stars 0 forks source link

[ALL] numPupilx/y and numImagx/y become Nx/y #69

Closed NKant13 closed 10 years ago

NKant13 commented 10 years ago

The parameters numPupilx/y and numImagx/y are always the same and will be changed into Nx and Ny to keep the nomenclature clean. So @larsvl, @ntielen, @yudhapane, @herminarto, @JaccovdS, @forcaeluz please be sure to change this in your functions. Also do not use numAperx/y and use instead the lensCentx/y to get the number of lenslets, this for robustness of the code.

Thanks!

---- EDIT ---- Please read through the conversation before taking this issue into account.

JaccovdS commented 10 years ago

So I can replace numAperx/y with lensCentx/y without any consequences?

faitdivers commented 10 years ago

With the size of lensCentx and lensCenty I guess.

On 9 June 2014 19:47, Jacco van der Spek notifications@github.com wrote:

So I can replace numAperx/y with lensCentx/y without any consequences?

— Reply to this email directly or view it on GitHub https://github.com/faitdivers/pyao/issues/69#issuecomment-45520579.

JaccovdS commented 10 years ago

Yes indeed I should be using the size. So for who might be needing it:

numAperx = len(lensCentx)
faitdivers commented 10 years ago

actually you can only retrieve numAperx*numApery from lensCentx, I think

On 9 June 2014 20:06, Jacco van der Spek notifications@github.com wrote:

Yes indeed I should be using the size. So for who might be needing it:

numAperx = len(lensCentx)

— Reply to this email directly or view it on GitHub https://github.com/faitdivers/pyao/issues/69#issuecomment-45523016.

NKant13 commented 10 years ago

You're right, using numAperx/y would be bad for the generalization of the code for other forms of lenslet setups.

JaccovdS commented 10 years ago

So in this test case it is a vector but in reality it would be a real array?

It would suffice to do

 numAperx, numApery = lensCentx.shape
faitdivers commented 10 years ago

I think it is a vector always. Each element of lensCentx contains the x-coordinate of te center of a lenslet.

On 9 June 2014 20:31, Jacco van der Spek notifications@github.com wrote:

So in this test case it is a vector but in reality it would be a real array?

— Reply to this email directly or view it on GitHub https://github.com/faitdivers/pyao/issues/69#issuecomment-45526329.

JaccovdS commented 10 years ago

Aha, now I see what you mean with your previous comment. In WFR I really need to know numAperx and numApery separately. So there has to be a way I can extract that otherwise we can't get rid of numAperx and numApery.

forcaeluz commented 10 years ago

But do you then assume a default lenslet configuration @JaccovdS?

faitdivers commented 10 years ago

A square mesh is considered in WFR. So I guess the best is, for now, to ask WFS to generate a square mesh as well.

On 9 June 2014 20:47, forcaeluz notifications@github.com wrote:

But do you then assume a default lenslet configuration @JaccovdS https://github.com/JaccovdS?

— Reply to this email directly or view it on GitHub https://github.com/faitdivers/pyao/issues/69#issuecomment-45528330.

JaccovdS commented 10 years ago

Yep we are not assuming a square but we are assuming an aligned lenslet configuration. I doesn't have to be square otherwise I could do a square root on the numAperx*numApery value. Edit: rectangle is the correct word ;)

faitdivers commented 10 years ago

Correct!

On 9 June 2014 20:50, Jacco van der Spek notifications@github.com wrote:

Yep we are not assuming a square but we are assuming an aligned lenslet configuration. I doesn't have to be square otherwise I could do a square root on the numAperx*numApery value.

— Reply to this email directly or view it on GitHub https://github.com/faitdivers/pyao/issues/69#issuecomment-45528723.

JaccovdS commented 10 years ago

On a second thought indirectly we are assuming a square matrix because we are taking the inverse. However that is actually a different discussion.

faitdivers commented 10 years ago

After the meeting we reached the following conclusion:

For the moment, we will use the noApertx/y. This means that we can only simulate aligned grids with an underlying rectangular, equidistant, regular, ..., lenslet grid configuration.

The variable lensCentx/y will still be used in WFS. There was some discussion about the format of this parameters (whether it should be a vector or an array) but no conclusions were drawn. @NKant13 and @amydeeb will decide on the most suitable way to implement this such that it is useful for everyone.

Notice that, as it is, lensCentx and lensCenty are two 1 dimensional vectors for which the pair (lensCentx(ii), lensCenty(ii)) yields the center of a circular lenslet in the pupil-plane. Thus if you take the length of the vector lensCentx/y you will get the Total number of lenslets.

We will also maintain numImagx/y and numPupilx/y although they will generally be the same. What does this mean? The number of samples/pixels in the pupil-/focal-plane will be the same. Why keep both if they are going to be the same? As we are working in two different planes it is important to differentiate between them and know exactly in which plane we're working in. It is also good for further generalisation, that is, if we want the sampling in one plane to be different from the other plane.

TL;DR: use noApertx/y for now; if possible use lensCentx/y once its format has been defined. numPupilx/y and numImagx/y will be maintained.

NKant13 commented 10 years ago

For now we keep lensCentx/y just as vectors.

faitdivers commented 10 years ago

Not relevant anymore. Maybe even confusing. So I'm going to close it.