xzos / PyZDDE

Zemax/ OpticStudio Extension using Python
MIT License
159 stars 67 forks source link

Rotation of written Beamfiles with Zemax July 11, 2011 #33

Closed xy124 closed 10 years ago

xy124 commented 10 years ago

if I execute the example beamFileWrite.py the Beamfile is rotated wrongly. That is probably because zemax sets the origin(0,0) to the left bottom corner and not to the left top as it's done for pixel coordinates normally. Furthermore I suggest to change

Ex_real[i][nx-j-1] = pix[i, j]

into

Ex_real[i][j] = pix[i, j]

in beamFileWrite.py bug_screenshot

indranilsinharoy commented 10 years ago

@xy124 @CatherineH

Hi xy124,

Thank you very much for pointing it out. This is actually a mistake made on my part, when I merged the submission from Catherine (her original code runs correctly). So, I looked at her original branch, and I think that the correct code should be the following:

for i in range(ny):
    for j in range(nx):
        Ex_real[nx-j-1][i] = pix[i, j]

I ran the example with the above code modification, and looked at the beamfile through the Zemax beamfile viewer. This is what I see: beamfile

Please let me know if you and Catherine think that this is indeed correct. I will make the appropriate changes after I hear back from you.

Thank you very much once again.

xy124 commented 10 years ago

ok now the format in ZEMAX is good(also my pikachu stands on its feet ;) ) But wouldn't it be better to choose a format of the electirc field like [Pixels_row1, Pixels_row2, ... Pixels_rowN] so that the beamfile will be rendered the same way in zemax as in the imshow of pylab? (I would implement it(as I need it nevertheless) and send a pull request if you agree) maybe try out this patch to see what I mean:

diff --git a/Examples/Scripts/beamFileWrite.py b/Examples/Scripts/beamFileWrite.py
index 2c2d471..e3aa8a8 100644
--- a/Examples/Scripts/beamFileWrite.py
+++ b/Examples/Scripts/beamFileWrite.py
@@ -16,12 +16,12 @@
 # Licence:   MIT License
 #-------------------------------------------------------------------------------
 from __future__ import print_function
 from PIL import Image
 from pyzdde.zdde import writeBeamFile, readBeamFile
 import matplotlib.pyplot as plt
 import os
 import time
-
+from pylab import *
 directory = os.path.dirname(os.path.realpath(__file__))
 im = Image.open(directory+"\pikachu2.png")

@@ -33,12 +33,16 @@ Ex_real = [[0 for x in range(nx)] for y in range(ny)]
 Ex_imag = [[0 for x in range(nx)] for y in range(ny)]
 Ey_real = [[0 for x in range(nx)] for y in range(ny)]
 Ey_imag = [[0 for x in range(nx)] for y in range(ny)]
-
 for i in range(ny):
     for j in range(nx):
-        Ex_real[i][nx-j-1] = pix[i, j]
+        #Ex_real[i][nx-j-1] = pix[i, j]
+        Ex_real[nx-j-1][i] = pix[i, j]
         #print(pix[i,j])

+# I'd like to change the code so that this whows me a good pikachu ;)
+
+imshow(Ex_real)
+show()
 n=(nx,ny)
 efield = (Ex_real, Ex_imag, Ey_real, Ey_imag)
 version = 0
indranilsinharoy commented 10 years ago

Hi @xy124, I am sorry for the late response. I had a very busy day today. Anyways, I like the idea of using imshow() to make the figure look more like the way it does in the beam file viewer. However, the figure will still be 90 degrees rotated in matplotlib figure, right? (unless we explicitly rotate it) Moreover, I would like to let @CatherineH decide on this change, as she implemented the beam file read/write functions, and created the original examples demonstrating these functions.

Regarding your comment

But wouldn't it be better to choose a format of the electric field like [Pixels_row1, Pixels_row2, ... Pixels_rowN]"

... I am not quite sure what you mean. I am already changed the statement inside the double for loop to Ex_real[nx-j-1][i] = pix[i, j] (thanks to you and Catherine). Apart of the line Ex_real[nx-j-1][i] = pix[i, j], I would like to hold off changing anything else right now (I hope you will appreciate). The modifications will appear in the master probably tomorrow, as I have implemented few more helper functions for reading MTF and PSF data and few other functions, and I am expecting to check in the code tomorrow.

On a different note, if you don't mind, I would like to add you to the contributors page. Please let me know if it is fine with you.

I will look forward to more pull requests from you in future :-).

Thank you and have a good weekend.

xy124 commented 10 years ago

I completely agree in waiting for @CaterineH to decide. As I'm using zbfWrite and -Read for my current work anyway I just need to know which conversations need to be done in my python scripts. If I watch Pikachu with the patch announced above, i get the following result:

tmp

Pikachu is flipped horizontal if I directly draw it with imshow(Ex_real). I'd like to change writeBeamFile and readBeamFile to load Ex_real-Arrays that are imshowed with correct orientation ;)

You're welcome to add me to contributors page :)

CatherineH commented 10 years ago

Hey guys,

The reason I had things the way they were was to demonstrate the difference in coordinate systems between standard image formats and zemax.

I would leave them the way they are.

xy124 commented 10 years ago

Well but normally it's much harder for people to recognize that the beamfile has wrong orientation than it is with Pikachu - the beamfiles I use have almost rotational symmetry . In my opinion one should either document this pretty well or make it more intuitive.

indranilsinharoy commented 10 years ago

Hi @xy124 , @CatherineH reading above, it seems to me that, may be, we should put a note (rather a "warning") about the difference in coordinate systems in the docstring, rather than changing the functions themselves.

@xy124, let me know what you think.

indranilsinharoy commented 10 years ago

Hi @xy124 and @CatherineH,

I just updated the master with several more functions (totally around 30). See the news and updates page for details.

I have also added "xy124" to the contributors page. Thanks for pointing out the bug in the example. I have updated the example file. Please let me know if the example works as expected. Please note that I have not added any "warning message" yet. I will wait for your inputs on that (and then close this issue).

Thank you all very much.