Antique.jl
Antique.jl copied to clipboard
Review for JOSS
This is in relations to the openjournals/joss-reviews#8545. Overall, I am very impressed Antique. It is a package that is very well put together and should come in useful whether for teaching, testing other codes, or further research.
I do have a few comments:
- Exporting single letter functions or orbjects is generally frowned upon in software engineering because it is likely to lead to name collisions and because it is not very informative. It is warranted within the package itself, but should be best avoided outside. From a teaching perspective, alphabet soups - even of the singular variety - are also a source of often unnecessary cognitive load.
Case in point: copy-pasting the initial example followed by second example in the REPL fails because
Eclashes withE. Other case in point: Are the wavefunctions Psi or psi? With quite a few fonts, it's difficult to tell. I believe a mixture of the following could remedy to the situation, though the authors are welcome choose to anything else that works for them: * do not export single letter objects. * in the examples, alwaysimport Antiquerather thanuse Antiqueand access single-letter objects by their full name * addwavefunction(x...; kw...) = \Psi(x...; kw...), and related aliases * it should be okay for the long-form aliases to be exported - The docs are generally very good and very detailled. It is a pleasure to see sources other than wikipedia cited in a code. However, the initial usage section of the documentation which is quite sparse. New users should not be expected to guess the symbols used for wavefunctions and potentials, nor how to access \psi and other greek letters from the terminal or editors of choice. A few extra lines in the Demonstration section would be welcome both to highlight the typography and other main functions.
- For those that come from a field where Legendre polynomials and friends rate up to n in the thousands, using a closed for of the polynomials will come as a surprise since it's not numerical stable. A few words highlighting up to which n and which accuracy the closed form and rodrigues' formula have been tested would be welcome.
- Could more details for figure 1 be given, especially with regards to the discrepancy between the numerical and exact forms, and perhaps how far the expansion is pushed. Is it expected that increasing the size of the expansion would resolve the discrepancy?
We appreciate your review. I opened some sub-issues for this review.
- The first comment points out the same issue as https://github.com/ohno/Antique.jl/issues/97. It should be resolved in v1.0.0.
- https://github.com/ohno/Antique.jl/issues/100 Some examples will be added with descriptions to demonstration section.
- https://github.com/ohno/Antique.jl/issues/101 I was not aware of numerical stability. We need to add tests with substituting concrete values, because the match between the closed‑form and Rodrigues’ formula is currently checked only by a computer algebra system, not with numerical values. However, there are no significant issues with the orthonormality test using numerical integration (from –∞ to ∞). The integration values are almost exact within the double‑precision range. Polynomials can overflow for large n due to factorial(21). For example, the associated Laguerre polynomials work only up to $n = 20$ in Int64, but work for larger n when using BigInt:
using Antique; H = HydrogenAtom(); Antique.L(H, 1.0, n=big"21"). - https://github.com/ohno/Antique.jl/issues/102 A more detailed explanation will be provided in the caption of Figure 1. As you mentioned, increasing the size of the expansion must resolve the discrepancy.