JOSS Review - Chris

[quietlychris]

Similar to Yuhan, I'm currently doing a first pass through the repository, and will dig in a little deeper soon:

• In general (and, frankly, unsurprisingly considering the history of BLAS/LAPACK within the Rust machine learning space), I found your section on differentiating between various BLAS backends to be a little confusing.
  • I think you might want to standardize your punctuation here--I thought for a moment that the BLAS heading was actually underneath persistent-moe, not Features.
  • I believe that LAPACK is an acryonym for "Linear Algebra PACKage," and is by convention typically all uppercase
  • Despite saying that, by default, egobox uses the pure Rust backend (it actually might be worth linking to linfa-linalg, and well as ndarray-linalg), there's at least a couple sections where your default instructions for running examples contradict this. For example in the ego module, your instructions say $ cargo run --release --example ackley --features linfa/intel-mkl-static, while in the main repository's README, you don't have mention of the intel-mkl-static. It's not seriously problematic, as both should work, but a little more consistency might help prevent a little confusion

[relf]

I agree. I've simplified and hopefully clarified this paragraph making the BLAS/LAPACK backend selection more an option. You can see the result here

[quietlychris]

When you run cargo test in the gp, you end up with a bunch of .npy files that just sit around in the project directory. Different people have different approaches for this, but I'd recommend that you instead test up those tests (and any examples) such that artifacts like those end up getting written into the target/ directory, where they'll be out of the way and won't risk accidentally getting committed by git or something.

Edit: this happens in moe and ego as well

[quietlychris]

It might be worth adding links in the primary egobox module's documentation to the actual subcrates--when you go to https://docs.rs/egobox, there's only a link to the Python version. They're definitely listed if you look under the dependency list on docs.rs, but it's not hard to imagine someone looking for the module documentation, search just for egobox instead of egobox-doe and then get confused about where the docs are. This approach is the same kind that Linfa uses--the top-level README links to all the subcrates just to make everything easy to find.

[quietlychris]

The JOSS submission checklist says:

Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

I suppose you could argue that simply being on Github offers a clear path forward on these, but since that's true on most JOSS projects and they asked for it anyway, I'm assuming they're probably interested in having a discrete Contributing section or something telling people that you will/won't accept PRs, author contact info (if you'd like to give it out, I know it's fairly common for "corresponding authors" to have their institutionally-affiliated address available)