faitdivers / pyao

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

[WFR] what should we return? #55

Closed JaccovdS closed 10 years ago

JaccovdS commented 10 years ago

What should we return because I don't think the initial matrix is correct, is it?

Check https://github.com/faitdivers/pyao/blob/iss32/src/WFR/mainWFR.py

In my opinion we should just return phi but this does not correspond to the shape of ones(...)

ntielen commented 10 years ago

I just took a quick look at the code. At least we miss the constants alpha/2*Dl which have to be multiplied with G. I am not sure if this fixes the problem tough. What we should return is a vector which contains all the values of phi at the phase points, but keep in mind they are reconstructed.

faitdivers commented 10 years ago

Coordinate with Hermin to see if he doesn't already do the conversion from centroids to slopes.

On 4 June 2014 21:13, ntielen notifications@github.com wrote:

I just took a quick look at the code. At least we miss the constants alpha/2*Dl which have to be multiplied with G. I am not sure if this fixes the problem tough. What we should return is a vector which contains all the values of phi at the phase points, but keep in mind they are reconstructed.

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

JaccovdS commented 10 years ago

@herminarto what do you provide? @forcaeluz what do you expect?

forcaeluz commented 10 years ago

The control part right now is just an PID. Before the PID there will be a mirror component. So you should ask the DM what they expect.

herminarto commented 10 years ago

I provide centroid and slope.
"Expected input: [s_x(0,0),...,s_x(0,N),...,s_x(M,0),...,s_x(M,N),s_y(0,0),...,s_y(0,N),...,s_y(M,0),...,s_y(M,N)]^T" means the slopes for subapertures right? So should I return that matrix?

JaccovdS commented 10 years ago

Yes if that is possible. Note that we actually expect a vector. For our test example we use: centroids = ones((sensorParameters['noApertx']_sensorParameters['noAperty']_2,1)) so that is the expected input. Used in the order you posted. Also see the lecture nodes because that is actually what we implemented.

@yudhapane what do you expect as input. See our code for what we are currently returning, is that ok?

JaccovdS commented 10 years ago

@herminarto The alpha term is already in the slopes am I right? I mean the alpha term that can be found on page 22 and 23 of the lecture notes.

faitdivers commented 10 years ago

Let me try to answer that: I believe the alpha term is already there. It's basically just dividing by the focal length.

On 6 June 2014 15:08, Jacco van der Spek notifications@github.com wrote:

@herminarto https://github.com/herminarto The alpha term is already in the slopes am I right? I mean the alpha term that can be found on page 22 and 23 of the lecture notes.

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

JaccovdS commented 10 years ago

Is it focal lenght or diameter? See: https://github.com/faitdivers/pyao/blob/master/src/main.py

faitdivers commented 10 years ago

If we are talking about the same think, it should be focal length:

https://github.com/faitdivers/pyao/blob/centroid/src/Centroid/findSlope.py

On 6 June 2014 15:55, Jacco van der Spek notifications@github.com wrote:

Is it focal lenght or diameter? See: https://github.com/faitdivers/pyao/blob/master/src/main.py

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

JaccovdS commented 10 years ago

Woops I am confusing two terms. So the alpha is already there. What I should do in WFR is an extra division by 2D_l which is the diameter I was talking about.

faitdivers commented 10 years ago

Yes. You need the 2D_l. That term has to do with the finite difference method and not with the slopes themselves.

On 6 June 2014 16:02, Jacco van der Spek notifications@github.com wrote:

Woops I am confusing two terms. So the alpha is already there. What I should do in WFR is an extra division by 2D_l which is the diameter I was talking about.

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

herminarto commented 10 years ago

Yes, the alpha is already there, since it's already divided by f (focal length)

JaccovdS commented 10 years ago

I still don't have an answer on this. I will just return phi in the way I have coded it now. Next one has to deal with it otherwise I can't never create a pull request for an implemented Fried geometry.

yudhapane commented 10 years ago

@JaccovdS : just return the phi the way you finished it now. I will try to adjust my part (DM) to your output. I will give you feedback asap. In what branch is your latest code now?

JaccovdS commented 10 years ago

The latest code is in iss32 https://github.com/faitdivers/pyao/tree/iss32

yudhapane commented 10 years ago

@JaccovdS: In order for DM to work successfully, we will need not only the reconstructed phase (I have tried your code and it is working fine), but also their spatial positions. I understand that this positions change according to the geometry. Therefore, if it is possible, I would also like to know the positions (x, y) please.

Thanks

JaccovdS commented 10 years ago

@yudhapane Do you mean the associated coordinates? So for example that the first phi has coordinate (0,0)? Or do you mean something different? Because I think at this moment that will be the same for Fried and Southwell. I still have to think about how to implement modified Hudgin because then it is indeed shifted.

yudhapane commented 10 years ago

@JaccovdS: what I mean is the spatial coordinates, not the index. Eg, if the first sample of Phi is at x = 0.000 and y = 0.000 so the next Phi perhaps at x = 0.001 and y = 0.000. If the geometry is fixed, I can calculate it myself as long as I know how the geometry looks like and how much is the spacing between wavefront samples. But the additional problem rises if the geometry changes.

JaccovdS commented 10 years ago

@yudhapane for Southwell and Fried it is just the index times the 'dl' where 'dl' is the distance between the lenslets.

# Distance between lenslets [m] 
    'dl' : 10.0e-6

So for Fried and Southwell you can just take the index of the phi and multiply it with 'dl'. I think there is no need for me to return an extra matrix with the spatial positions, is there? I will look into returning a spatial positions matrix when constructing modified Hudgin.

forcaeluz commented 10 years ago

@JaccovdS: I think that if you are going to send it for one geometry, send it for all the geometries, to keep consistency.

JaccovdS commented 10 years ago

@forcaeluz what I meant is that I will implement it at the moment I will implement modified Hudgin (for all three geometries). But I can already do it now however it is very trivial.

JaccovdS commented 10 years ago

I think that for the moment @yudhapane can just use this part from the paramsSensor

    # Compute lenslet centres and check minimal array widths 
    lx, ly, lensCentx, lensCenty = lensletCentres(paramsSensor)
    # Normalized lenslet centers
    paramsSensor['lensCentx'] = lensCentx
    paramsSensor['lensCenty'] = lensCenty
    # Set correct array widths
    paramsSensor['lx'] = lx
    paramsSensor['ly'] = ly

Can someone confirm this?

faitdivers commented 10 years ago

But if you're using Fried you will have the following:

o o | x | o o

where 'x' represents the lenslet centre and 'o' the phase points. So they don't match.

To generate the 'o's is not difficult. Just some shifting has to occur and adding a row and a column of phi's.

JaccovdS commented 10 years ago

@yudhapane check #82 for the resulting code