faitdivers / pyao

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

Open loop simulation #23 #50 #51

Closed forcaeluz closed 10 years ago

forcaeluz commented 10 years ago

Fixed the open-loop simulation, and also added time simulation to it. This should then close two issues. ( #23 and #50 )

faitdivers commented 10 years ago

Note that I can't automatically merge now. Please solve the conflicts with the master for the next pull request iteration.

forcaeluz commented 10 years ago

Yeah, there were no conflicts this morning, before the merge from pull request #42. Will take a look at the conflicts tonight.

forcaeluz commented 10 years ago

@faitdivers: I there was an comment about the follow order of the simulation. My question is, do you agree with my comment about why to keep it as it is?

faitdivers commented 10 years ago

I saw your point. But it would be nice to see the effects of the DM even in open-loop. I believe that in your implementation the DM in open-loop is virtually useless, right?

forcaeluz commented 10 years ago

Not really. As it is now it is useless as we don't actuate it. But if we actuate it, it will influence the simulation like it does on the closed-loop.

faitdivers commented 10 years ago

I see your point. But if you want to study the performance of the DM in open-loop, you basically can't.

The final decision is up to you. What I would suggest (and it's purely personal taste) is that you actuate the mirror but simply do not apply the correction. Does that make sense? I am perfectly aware that it does not make sense from a 'physical' point of view, but from a simulation and academic perspective I think it's okay.

As I said, this is is purely personal. I would say to ask somebody else for solving the dispute.

forcaeluz commented 10 years ago

I will keep it open, to be discussed in the next meeting.

faitdivers commented 10 years ago

I see no problem in merging it as it is. Really. If someone else complains we can create an issue for it. It's really just a minor detail which can be very easily fixed.

On 7 June 2014 16:25, forcaeluz notifications@github.com wrote:

I will keep it open, to be discussed in the next meeting.

— Reply to this email directly or view it on GitHub https://github.com/faitdivers/pyao/pull/51#issuecomment-45411330.

JaccovdS commented 10 years ago

As suggested by @forcaeluz I think it is good to discuss the working of DM at the next meeting. For now I will just merge this pull request.