search

ngedwin98 / ABCDBeamTrace.jl

ABCD formalism for linear optics in Julia: ray transfer matrices and Gaussian beam propagation
Other
9 stars 3 forks source link

Further Development Directions #6

Open ghost opened 5 years ago

ghost commented 5 years ago

To make the most of this package, I'd like to see

  1. Integration with Unitful. This may require changing the expected number type from Real to Number.
  2. Type-parametrization of the types Beam (for performance, and because it could easily be done in conjunction with 1)
  3. Documented examples for the use with symbolic numbers; see this discussion for possible packages to provide it. Integration work if that is required.
  4. Deprecating confusing names (at least spotsize which returns the beam's waist radius, not a diameter as the name suggests).
  5. Possibly rename the package to something less application-specific, e.g. to RayTransferMatrices.jl I also think "ABCD" clashes with the (proper) capitalization of "Beam", as well as with julia's guideline to avoid subject-specific abbreviations.
  6. Web-published documentation (with function signatures) for easy reference.
  7. (optional) integration into julia's package manager

Are these goals agreeable, or would you, @ngedwin98, recommend that I maintain a fork of this package to make that suit my needs? I have no particular ambitions to do that but since I feel I need at least most of the above, I might do so if you wish to take this package in a different direction.

ghost commented 5 years ago

A quick note: The idea of point 3 is to be able to numerically optimize a system; the most common application might be cavity (optical resonator) design.

ghost commented 5 years ago

Also, this is in addition to plotting a beam going through a system, as #5 introduces along with documentation generation.

ghost commented 5 years ago

I'm going ahead with implementing some of these issues in https://github.com/Quantum-Factory/ABCDBeamTrace.jl/, e.g. with https://github.com/Quantum-Factory/ABCDBeamTrace.jl/pull/2 for adding docstrings (as well as minor other changes). I hope we can merge that back in here some day!

ngedwin98 commented 5 years ago

I really appreciate these ideas being brought up, as they sound like they represent what might be needed in an actual application setting for a package like this.

First, to address the issues that, in my impression (and correct me if I'm wrong), are more critical towards having a functional set of code for your application:

  1. I've considered but didn't end up using units since I reasoned that there's only one unit (i.e., a length scale) in the entire domain of the package. But I've noticed since then even then, units can be useful when working across many orders of magnitude, as in the micron-scale beam waists to the meter-scale propagation lengths. So, I do support a clean integration with units (including type-parameterization for performance), pending a resolution to ajkeller34/Unitful.jl#223.

  2. I'm not a huge fan of, for example, having 6+ type parameters for Beam; it makes any error messages rather verbose (I'm thinking of my experience with DifferentialEquations.jl here.). Perhaps we can force all fields within the type to have the same concrete type during construction (at least for all the real parameters).

  3. If possible, I'd like integration with symbolics to be implicit rather than needing to add code especially for interfacing with that. Perhaps you can elaborate on your use case? Along these lines, I am also enthusiastic about adding a similar level of (implicit) support for things like automatic differentiation (e.g., via https://github.com/JuliaDiff/ForwardDiff.jl).

Then, regarding additional development directions towards a more publicly-useful package:

  1. Would spotradius be more agreeable? I am still ambivalent with regards to much of the naming convention. My approach thus far has been to use semantically meaningful names with the exported API, and more field-specific shorthand (e.g., λ, z) in unexported variables/functions.

  2. I agree the naming needs to be given serious thought, upon integration with Julia's package manager. I do want to emphasize the Gaussian-beamtracing nature of the project (as opposed to just ray transfer matrices).

  3. I am in support of documentation as well, at least of the external API. I see that you've started to use Documenter.jl in #5. Operationally, what fraction of the way does that take us towards this goal?

  4. This ought to be done as soon as possible, pending most of these issues and a stable interface. Having thought about this package again for the first time in a while, there are also a few changes I'm considering. I'll open appropriate issues about them soon. I think once the API is stable, we can provide a release (which has the benefit of potentially providing a doi for the package for academic citation). After the release, I think it is time to integrate with the package manager.

ghost commented 5 years ago

Thanks for being so receptive!

  1. What are your thoughts on a very permissive type parametrization, i.e. allow Number or even Any to be the underlying datatype? That would have the advantage of being able to integrate with symbolic computation packages that might represent symbols as a Symbol rather than a Number (which may not be correct; I haven't investigated very deeply yet).

  2. [EDIT]: Absolutely, that was the idea, to only have 1 or at most 2 types involved. [EDIT: A related issue to 2.]: I absolutely agree that too many positional arguments create confusion and are error-prone. In fact, I already have difficulty with 2 parameters, especially since R and (theta) seem to interchange positional places between e.g. Mirror and Interface. I have hence changed all those to keyword arguments in my fork of your repo, with the intention of, in the future, allowing beam creation with more (but named) arguments.

  3. I understand. See 1.

  4. Absolutely: I'm warming up to spotradius and will change waistradius (and waistradiusfunc, in case it can be kept) to use that.

  5. Thank you for agreeing. However, I would not emphasize the Gaussian nature because the RTM could be useful in another context, too. But of course the choice is yours.

  6. If you want to see how far docs are along (reasonably far, IMHO, with 2 example plots), please clone my working fork https://github.com/Quantum-Factory/ABCDBeamTrace.jl and build the documentation with include("docs/make.jl"); the result will be in docs/build/ (open index.html).

  7. I agree that the interface should be (at least somewhat) stable first. Note, however, that all my changes should continue to allow your old syntax because I only added deprecation warnings (that also inform of the new syntax).