Gaussian optics module #617
Conversation
This commit implements gaussian optics module. To import use sympy.physics.gaussopt. It implements Ray transfer matrices for geometrical rays and gaussian beams. Conjugation relations. Doctests are present. Unit tests are not present.
|
Should there be a separate test_gaussopt.py file? I see that the functions all have nice doctests. Copying these over would be helpful. |
|
I have checked the spelling and your remark about NotImplementedError is correct. I'm adding it shortly. |
This commit implements gaussian optics module. To import use sympy.physics.gaussopt. It implements Ray transfer matrices for geometrical rays and gaussian beams. Conjugation relations. Doctests are present. Unit tests are not present.
|
Concerning separate test file test_gaussopt.py: should there be one if it only mimics the doctests? I don't think so. Still I can imagine some additional tests but they seem redundant. |
|
I prefer tests to doctests because I always check the tests and rarely run the doctests. This is a failing on my part to be sure but I get the impression that it's common. |
|
The following lines/routines need tests I would recommend renaming R, dist and ang to radius, distance and angle. I put the doctests into a test suite and sent this as a pull request. Do you know how to merge it into your branch? |
|
I merged the tests. Thanks for the work (have you used a script to transform the tests?). Renaming and testing those routines is on my todo list. |
|
You should definitely have tests in addition to doctests. The doctests are just examples, which happen to be tested so that they are correct. The unit tests are the ones that are used to check for regression. So, for example, the test coverage does not even consider the doctests, and it should be assumed that any doctest can be changed without worrying about changing coverage, e.g., if the example is more enlightening. |
|
editor magic and elbow grease |
| CurvedRefraction, FlatMirror, FlatRefraction, FreeSpace, GeometricRay, | ||
| FreeSpace, RayTransferMatrix, ThinLens, ThinLens, conjugate_gauss_beams, | ||
| gaussian_conj , geometric_conj_ab, geometric_conj_af, geometric_conj_bf, | ||
| rayleigh2waist, waist2rayleigh) |
There was a problem hiding this comment.
I would put an empty line here, but that's minor.
|
Test results html report: http://pastehtml.com/view/b8ffy651i.html Summary: There were test failures. @Krastanov: Please fix the test failures. Automatic review by sympy-bot. |
| and args[0].shape == (2,2): | ||
| temp = args[0] | ||
| else: | ||
| raise Exception('Bad arguments for the constructor.') |
There was a problem hiding this comment.
The code quality tests seem to fail here.
There was a problem hiding this comment.
Something like this can be used:
raise ValueError('Expecting 2x2 Matrix or the 4 elements of the Matrix but got %s' % args)
|
I have added the last tests suggested by smichr. I have also fixed the code quality errors. doctests, test_gaussopt, and test_code_quality pass. |
|
Just a question about the ray angle. I'm not sure how to think about that if I have a sequence of items through which the beam is passing, e.g. through two lenses. Are the lenses always perpendicular to the beam? If so, the only time I would imagine the beam angle mattering is when a mirror or refractive surface is hit. But in those cases, my first thought is that I would give them an orientation angle, not the beam. |
|
I'll check the error messages. Concerning the formalism: for example if we have a ray at angle alpha to the axis and passing at height h over the axis, after distance d in free space the angle will be the same, but the height of the ray will be alpha*d + h. All angles are considered small (gaussian approximation) |
|
I've fixed the error message and squashed the last commit. |
|
Test results html report: http://pastehtml.com/view/b8hdxumpj.html Summary: There do not appear to be any problems. Please double check this against the test report given above before merging with master. Automatic review by sympy-bot. |
|
tests for height, angle, and multiplication of two RayTransferMatrix are missing. |
|
I've added those to the TODO. |
No general exceptions in the code. Tests for conjugation at distance == oo. Tests for multiplication of RayTransferMatrices. Tests for methods of GeometricalRay.
|
I've squashed the last commit addressing the remarks of @smichr. Unittests are present.
code_quality, doctest and test_gaussopt all pass. I've checked the raised exceptions. |
|
Test results html report: http://pastehtml.com/view/b912u79pq.html Note A custom test command was used: python2.5 setup.py test Summary: There do not appear to be any problems. Please double check this against the test report given above before merging with master. Automatic review by sympy-bot. |
|
Thanks, this is in! (I still remember the humiliation as a grad student of being given an exercise in tracing a ray through a series of optical pieces and failing miserably. The prof's introduction of sign conventions the next day was like water for a thirsty man :-)) |
This commit implements gaussian optics module.
Announced on the mailing list: https://groups.google.com/d/topic/sympy/nDG4i32GJF0/discussion
It implements
Ray transfer matrices for geometrical rays and gaussian beams.
Conjugation relations.
Not all functions have been used in practice, so errors in the implemented formulas are still possible. That being said, I have not found any.
Doctests are present.
Unittests are present.
code_quality, doctest and test_gaussopt all pass.
I've checked the raised exceptions.
TODO: final review? or additional fixes...