Open ngedwin98 opened 5 years ago
Just my two cents: I consider this interface in its current form acceptable because not applying either Tan
or Sag
should result in behavior that is (in some perhaps only vaguely defined way) "intermediate" between the two cases.
It could be debated whether we instead want to have it evaluate (e.g. the associated Ray Transfer Matrix) to NaN
rather than the current "intermediate value between sagittal and tangential."
A related issue: The capitalization of RTM
is against julia's naming convention. Should it be changed into rtm
, matrix
(my favorite), or perhaps even Base.Matrix
? The latter would not constitute type piracy if done for a package's own types and would allow promotion (after defining a promote_rule
) in case we ever want to allow e.g. simply writing e.g. FreeSpace(...) * ThinLens(...)
to get the combined ray transfer matrix.
At the moment, the composite elements
Tan
andSag
allow for dispatch to the appropriate ray transfer matrices for tangential and sagittal components, but they make use of angle information stored inElement.θ
. Semantically, this can lead to somewhat confusing situations; viz., the line in the README that says:"Note: If no calls are made to either
Tan
andSag
[...], theθ
field (if present) confers no effect; in this case,RTM
returns the ray transfer matrix as if we setθ=0
."On the other hand, it is undesirable to have to reconstruct the same optical system, specifying each time whether we want the tangential or sagittal components. A straightforward refactoring of the angle information out of the basic optical elements and into the construction of the
Tan
andSag
elements instead would force this undesirable workflow.This issue intends to solicit a clever workaround that might satisfy both requirements somehow, in the hopes of resolving the issue before a code release.