search

faitdivers / pyao

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

Changes to mainWFG, added Kolmogorov & vonKarman #77

Closed larsvl closed 10 years ago

larsvl commented 10 years ago

Some additions and changes to operate the new functionality in WFG.

forcaeluz commented 10 years ago

:+1: Although the object oriented part is not how I would have done it, the code looks very neat, with well defined, small and easy to understand functions. I cannot test right now, and I'm not really sure about the correctness, as I don't know all the theory behind it.

For me this is ok, but I think it is necessary that someone else also checks it. And I will test it tonight.

larsvl commented 10 years ago

Thanks, but then again there are so many different ways, plus it is (somewhat) in line with the ZernikeWave class.

----- Reply message ----- From: "forcaeluz" notifications@github.com To: "faitdivers/pyao" pyao@noreply.github.com Cc: "larsvl" larsvl@live.nl Subject: [pyao] Changes to mainWFG, added Kolmogorov & vonKarman (#77) Date: Wed, Jun 11, 2014 15:58

:+1: Although the object oriented part is not how I would have done it, the code looks very neat, with well defined, small and easy to understand functions. I cannot test right now, and I'm not really sure about the correctness, as I don't know all the theory behind it.

For me this is ok, but I think it is necessary that someone else also checks it. And I will test it tonight.

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

forcaeluz commented 10 years ago

To be quite honest, I'm not really sure how I would have done it either. A small example: (This are only suggestions, you don't need to implement them, as object oriented programming is not the goal in this project, and if it becomes necessary, we can refactor later) The phaseScreen for instance has it's code mainly divided it two parts: Or it does something for Kolmogorov, or von Karman. If you have one class of type phaseScreen wich basically only contains the shared code between Kolmogorov and von Karman, and two subclasses (kolmogorov and von Karman), you can also give the classes named paramters, instead of the shared param[] array.

But again, you don't need to act on this. And yes, there are many different ways to define objects.

forcaeluz commented 10 years ago

One question: Do you also have testing code for this?

larsvl commented 10 years ago

To reply to the first message: indeed the code currently consists of two parts, because i've limited it to two types of aberrations. Personally, envisioned something in the future, with a WFG class, where you can add these aberrations to, a parent-child structure or simply one class that contains it all, more or less making wfg(...) useless, but thus has a low priority now, as you correctly noted.

To answer your second question: no there is no test code yet, as I wish to solve the issue of the relative paths to the reference data first.

----- Reply message ----- From: "forcaeluz" notifications@github.com To: "faitdivers/pyao" pyao@noreply.github.com Cc: "larsvl" larsvl@live.nl Subject: [pyao] Changes to mainWFG, added Kolmogorov & vonKarman (#77) Date: Wed, Jun 11, 2014 20:23

One question: Do you also have testing code for this?

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

forcaeluz commented 10 years ago

And another question:

I want to be able to check more than only if the code runs or not, but also what it is outputting. Is there somewhere where I can look for plots and expected plots?

larsvl commented 10 years ago

To answer your question, yes there is! Just set the debug flag on true like wfg(..,..,..,True). Alternatively, use the instance of the class and put plotWavefront() behind it.

If you have any suggestions on the data thing I would be grateful!

----- Reply message ----- From: "forcaeluz" notifications@github.com To: "faitdivers/pyao" pyao@noreply.github.com Cc: "larsvl" larsvl@live.nl Subject: [pyao] Changes to mainWFG, added Kolmogorov & vonKarman (#77) Date: Thu, Jun 12, 2014 11:30

And another question:

I want to be able to check more than only if the code runs or not, but also what it is outputting. Is there somewhere where I can look for plots and expected plots?

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

forcaeluz commented 10 years ago

Look at the last commit. This should be quite safe.

The data loads, but the wafe-front test fails. Is the data wrong, or is there somewhere else a problem?

larsvl commented 10 years ago

It fails to load the data, but it has an odd traceback in the class...

----- Reply message ----- From: "forcaeluz" notifications@github.com To: "faitdivers/pyao" pyao@noreply.github.com Cc: "larsvl" larsvl@live.nl Subject: [pyao] Changes to mainWFG, added Kolmogorov & vonKarman (#77) Date: Thu, Jun 12, 2014 14:42

Look at the last commit. This should be quite safe.

The data loads, but the wafe-front test fails. Is the data wrong, or is there somewhere else a problem?

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

forcaeluz commented 10 years ago

Can you write that for me? Here it only shows that the assertion fails.

larsvl commented 10 years ago

%tb

SystemExit Traceback (most recent call last) ~\AppData\Local\Enthought\Canopy\App\appdata\canopy-1.3.0.1715.win-x86_64\lib\site-packages\IPython\utils\py3compat.pyc in execfile(fname, glob, loc) 195 else: 196 filename = fname --> 197 exec compile(scripttext, filename, 'exec') in glob, loc 198 else: 199 def execfile(fname, *where):

~\GitHub\pyao\src\WFG\testWFG.py in () 67 68 if name == "main": ---> 69 unittest.main()

~\AppData\Local\Enthought\Canopy\App\appdata\canopy-1.3.0.1715.win-x86_64\lib\unittest\main.pyc in init(self, module, defaultTest, argv, testRunner, testLoader, exit, verbosity, failfast, catchbreak, buffer) 93 self.progName = os.path.basename(argv[0]) 94 self.parseArgs(argv) ---> 95 self.runTests() 96 97 def usageExit(self, msg=None):

~\AppData\Local\Enthought\Canopy\App\appdata\canopy-1.3.0.1715.win-x86_64\lib\unittest\main.pyc in runTests(self) 232 self.result = testRunner.run(self.test) 233 if self.exit: --> 234 sys.exit(not self.result.wasSuccessful()) 235 236 main = TestProgram

SystemExit: True

Date: Thu, 12 Jun 2014 07:52:16 -0700 From: notifications@github.com To: pyao@noreply.github.com CC: larsvl@live.nl Subject: Re: [pyao] Changes to mainWFG, added Kolmogorov & vonKarman (#77)

Can you write that for me? Here it only shows that the assertion fails.

— Reply to this email directly or view it on GitHub.

forcaeluz commented 10 years ago

How are you calling this file?

larsvl commented 10 years ago

By running it from the working directory.

----- Reply message ----- From: "forcaeluz" notifications@github.com To: "faitdivers/pyao" pyao@noreply.github.com Cc: "larsvl" larsvl@live.nl Subject: [pyao] Changes to mainWFG, added Kolmogorov & vonKarman (#77) Date: Fri, Jun 13, 2014 00:50

How are you calling this file?

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

forcaeluz commented 10 years ago

Via canopy ide? What if you run it from command line? python testWFG.py

faitdivers commented 10 years ago

In Debug mode, perhaps because of scale problems, I can only see a flat wf when I simulate the vonKarman model.

faitdivers commented 10 years ago

@forcaeluz These are the errors that I get:

ERROR: testNollMapping (main.TestZernikeWavefront)

Traceback (most recent call last): File "WFG/testWFG.py", line 68, in testNollMapping data = load(os.getcwd()+'\Documents\GitHub\pyao\src\WFG\indexTestData.npy') File "/Users/jsilva/Library/Enthought/Canopy_64bit/User/lib/python2.7/site-packages/numpy/lib/npyio.py", line 367, in load fid = open(file, "rb") IOError: [Errno 2] No such file or directory: '/Users/jsilva/Desktop/PyAO/src\Documents\GitHub\pyao\src\WFG\indexTestData.npy'

ERROR: testWavefront (main.TestZernikeWavefront)

Traceback (most recent call last): File "WFG/testWFG.py", line 45, in testWavefront self.assertEqual(data,wf) File "/Applications/Canopy.app/appdata/canopy-1.3.0.1715.macosx-x86_64/Canopy.app/Contents/lib/python2.7/unittest/case.py", line 515, in assertEqual assertion_func(first, second, msg=msg) File "/Applications/Canopy.app/appdata/canopy-1.3.0.1715.macosx-x86_64/Canopy.app/Contents/lib/python2.7/unittest/case.py", line 505, in _baseAssertEqual if not first == second: ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

Ran 4 tests in 0.082s

FAILED (errors=2)

In the first one, the creation of the path is not OK. It tries to append to the path of my working directory. The second one seems to have only to do with the way the assert was made.

forcaeluz commented 10 years ago

Then not all of my changes were commited. The testNollMapping() function should have a similar way of importing data as the testWavefront() function. I though I commited that as well. I can commit this tomorrow in the morning. But I think that shouldn't be holding this pull request as this is not what this branch intended to fix. A new issue can be made to fix and add unit tests for the WFG.

On Sun, Jun 15, 2014 at 12:00 PM, João Lopes e Silva < notifications@github.com> wrote:

@forcaeluz https://github.com/forcaeluz These are the errors that I get: ERROR: testNollMapping (main.TestZernikeWavefront)

Traceback (most recent call last): File "WFG/testWFG.py", line 68, in testNollMapping data = load(os.getcwd()+'\Documents\GitHub\pyao\src\WFG\indexTestData.npy') File "/Users/jsilva/Library/Enthought/Canopy_64bit/User/lib/python2.7/site-packages/numpy/lib/npyio.py", line 367, in load fid = open(file, "rb") IOError: [Errno 2] No such file or directory: '/Users/jsilva/Desktop/PyAO/src\Documents\GitHub\pyao\src\WFG\indexTestData.npy' ERROR: testWavefront (main.TestZernikeWavefront)

Traceback (most recent call last): File "WFG/testWFG.py", line 45, in testWavefront self.assertEqual(data,wf) File "/Applications/Canopy.app/appdata/canopy-1.3.0.1715.macosx-x86_64/Canopy.app/Contents/lib/python2.7/unittest/case.py", line 515, in assertEqual assertion_func(first, second, msg=msg) File "/Applications/Canopy.app/appdata/canopy-1.3.0.1715.macosx-x86_64/Canopy.app/Contents/lib/python2.7/unittest/case.py", line 505, in _baseAssertEqual if not first == second: ValueError: The truth value of an array with more than one element is

ambiguous. Use a.any() or a.all()

Ran 4 tests in 0.082s

FAILED (errors=2)

In the first one, the creation of the path is not OK. It tries to append to the path of my working directory. The second one seems to have only to do with the way the assert was made.

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

faitdivers commented 10 years ago

In Debug mode, the Kolmogorov looks like a tilt. It should look like this:

forcaeluz commented 10 years ago

The testWavefront() function now generates a test failure, instead of error. The files are loaded correctly now, so the problem is either the data in the file, or the data generated by the zernike functions.

larsvl commented 10 years ago

The kolmogorov and von karman aberration still act strange. Altough l, I did find a bug in the code, there should be a minus in the creategrid function call. I failed to reproduce a plot like João gave. Still the code now seems to match the results in matlab phase screen example on Blackboard. Might it be as simple as supplying the right parameters? Or is there some other approach on this matter then is given in the book "Numerical Simulation of Optical Wave Propagation" on page 168-171?

----- Reply message ----- From: "João Lopes e Silva" notifications@github.com To: "faitdivers/pyao" pyao@noreply.github.com Cc: "larsvl" larsvl@live.nl Subject: [pyao] Changes to mainWFG, added Kolmogorov & vonKarman (#77) Date: Sun, Jun 15, 2014 17:29

In Debug mode, the Kolmogorov looks like a tilt. It should look like this:

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

forcaeluz commented 10 years ago

Can you commit the fix for the bug already?

larsvl commented 10 years ago

I think I have the bug why it is not giving the proper results, I haven't implemented the high frequency components of the kolmogorov aberrations yet.

----- Reply message ----- From: "forcaeluz" notifications@github.com To: "faitdivers/pyao" pyao@noreply.github.com Cc: "larsvl" larsvl@live.nl Subject: [pyao] Changes to mainWFG, added Kolmogorov & vonKarman (#77) Date: Mon, Jun 16, 2014 10:07

Can you commit the fix for the bug already?

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

forcaeluz commented 10 years ago

There is some logic in it! Do you think you can get it working today?

larsvl commented 10 years ago

Before the presentation will be difficult, but today should possible.

----- Reply message ----- From: "forcaeluz" notifications@github.com To: "faitdivers/pyao" pyao@noreply.github.com Cc: "larsvl" larsvl@live.nl Subject: [pyao] Changes to mainWFG, added Kolmogorov & vonKarman (#77) Date: Mon, Jun 16, 2014 10:11

There is some logic in it! Do you think you can get it working today?

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

faitdivers commented 10 years ago

I'm getting this error in the Zernike modes. Do you get this as well @larsvl ?

FAIL: testWavefront (main.TestZernikeWavefront)

Traceback (most recent call last): File "testWFG.py", line 45, in testWavefront self.assertTrue(test_result, 'Generated wave-front is different than expected.') AssertionError: Generated wave-front is different than expected.

Ran 4 tests in 0.019s

FAILED (failures=1)

larsvl commented 10 years ago

Can we pull this branch request to the master already? I believe I'm not the one here in the best position to judge this, besides I've created the pull request.

forcaeluz commented 10 years ago

Can you take a look at the failing test? The files are loading correctly now (at least it loads here and for faitdivers as well).

larsvl commented 10 years ago

The test fails becaause it loaded old data with a circular aperture. This was removed by request of the WFR guys but I failed to replace the data binary. It took me a while to figure out, as canopy still was issues with finding the binary. Hopefully this solves the issues so we can merge.

forcaeluz commented 10 years ago

Test now works. @faitdivers: Are the changes to the high frequency Kolmogorov and Von Karman correct now? If this branch is ok for you, it can be merged.

faitdivers commented 10 years ago

Yap, they were fine last time I checked.