Closed bddonovan closed 5 years ago
Hey Ben! I accepted your merge. Thanks for still contributing. Meanwhile, I've been making changes here and there as I work on spectrometer optical design. I didn't do the usual branch and merge, I just pushed the changes directly to the master.
One change that might screw up your code is that I swapped a sign convention in radGrat. This was an ad hoc change to get a test working in some of my code.
I've actually never been happy with how radGrat is implemented. It requires a lot of keeping track of signs on the part of the user. I suspect that switching to arctan2 as opposed to arctan will fix a lot of this. It's also annoying that you have to do a coordinate transformation to the hub before you call the function. I did put in a radgratcenter that you can call from the center of the grating (as opposed to at the hub).
So the radgrat functions probably need to be cleaned up. What do you think the best way forward is?
Ryan
Hi Ryan,
No problem. This is being used for the raytrace of the revamped OGRE payload which is why I’m continuing to use it.
I did notice the sign convention change in radgrat, but it wasn’t to hard to fix. Like you said, it was just a sign change.
And finally, I’ve gotten used to the way radgrat works currently, so it doesn’t bother me. However, I just recently tried to teach PyXFocus to some other people in Randy’s lab for a project and they got thoroughly confused at all the transformations required to implement the radgrat function. I’m not sure I know the best way forward. Let me give it a think!
Best, Ben
From: rallured notifications@github.com Sent: Saturday, April 6, 2019 8:31 AM To: rallured/PyXFocus PyXFocus@noreply.github.com Cc: Donovan, Benjamin David bdonovan@psu.edu; Author author@noreply.github.com Subject: Re: [rallured/PyXFocus] Made functionality of grat & radgrat agree + other small stuff (#8)
Hey Ben! I accepted your merge. Thanks for still contributing. Meanwhile, I've been making changes here and there as I work on spectrometer optical design. I didn't do the usual branch and merge, I just pushed the changes directly to the master.
One change that might screw up your code is that I swapped a sign convention in radGrat. This was an ad hoc change to get a test working in some of my code.
I've actually never been happy with how radGrat is implemented. It requires a lot of keeping track of signs on the part of the user. I suspect that switching to arctan2 as opposed to arctan will fix a lot of this. It's also annoying that you have to do a coordinate transformation to the hub before you call the function. I did put in a radgratcenter that you can call from the center of the grating (as opposed to at the hub).
So the radgrat functions probably need to be cleaned up. What do you think the best way forward is?
Ryan
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://nam01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Frallured%2FPyXFocus%2Fpull%2F8%23issuecomment-480500352&data=02%7C01%7Cbdonovan%40psu.edu%7C1afdfeb8967b4988c26608d6ba8bba86%7C7cf48d453ddb4389a9c1c115526eb52e%7C0%7C0%7C636901506665036174&sdata=TN3L79z9ctZC5LNnfesK0v0sP%2FXcidQfJ8QnIPiHJDs%3D&reserved=0, or mute the threadhttps://nam01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAbqknKBI9XwzJy-kbCUwwJ1BD9BTU9wuks5veJOCgaJpZM4cfe6p&data=02%7C01%7Cbdonovan%40psu.edu%7C1afdfeb8967b4988c26608d6ba8bba86%7C7cf48d453ddb4389a9c1c115526eb52e%7C0%7C0%7C636901506665036174&sdata=7g8DNB0kX4MGIYmUJ%2BcEHbCDMxky9VhHYiC%2BYFyNvbM%3D&reserved=0.
I'll end up fixing radgrat next weekend (I am cranking out grating spectrometer optical design analysis on the weekends). I think I can use arctan2 and the largest direction cosine to automatically figure out which way to translate the hub. For instance, if rays[5] is large and positive, then the hub should go off in the positive y direction. This leads to a function that can be called from the center of the grating, without needing to rotate such that the rays are pointing in any certain way.
grat didn't preserve the sign of the z direction cosine, while radgrat did. I've now fixed this, plus added stuff to .gitignore and some additional comments.