[JOSS Review] recommended revisions

[MilesCranmer]

Hi all,

Here is my review for JOSS (https://github.com/openjournals/joss-reviews/issues/3199), apologies for the delay. Here is my current recommended list of revisions. I will post additional messages here with further comments as we go along.

Overall:

• [x] It is a bit unclear who the audience is. Is this: people who do both ML and turbulence? Or, people who do turbulence, but have never tried ML? For those who do both, you may want to present this as a "model zoo" rather than a pipeline, since presumably those who already do ML will have their own pipelines set up and would want to plug in your models, or use your project for baseline modelling. For people who do turbulence but not ML, I think this seems very useful so they don't have to learn the internals of how ML works. But I think, for that audience, there is too much jargon here, and you should consider what changes you can make so that someone completely new to ML would be able to use your project for science.
  • I should mention: for another project (workshop version here: https://simdl.github.io/files/26.pdf, but longer one is in the works) - we ran baselines for different turbulence models, and actually Kim & Alvaro had to implement each one themselves. But it would have been much better if there was a "model zoo" that we could just run automatically, like with Sapsan. I think that would be a really useful area you could satisfy, as more and more people are working on ML & Turbulence.
• [x] README and goals of the project seem much too broad. I think you should focus in on exclusively turbulence modeling problems. Don't aim for generic usage - the feature creep will hurt your project, and prevent you from tuning parts of your software to this specific area which would make it better for your user base. There are far far too many "easy ML pipeline" projects out there, many of them backed by large corporate entities, so I would not try to satisfy that need. Emphasize that your software targets the unsaturated area---ML for turbulence! And really try to make it optimized for that specific problem, in a way that the generic pipelines can't.
  • Take a look through https://github.com/josephmisiti/awesome-machine-learning#python and you can see just how saturated this area has become. I really think it is better to specialize and emphasize your software is better for your particular science, compare to more generic tools.

Code review:

• [x] The software requirements are much too specific. I would change them all to be >=, and only if you know of a specific reason to keep it fixed to a version should you change it. Right now when I install it on Google Colab and my base env, I am unable to satisfy version requirements without downgrading my packages.
• [x] Can change git clone -> "pip install git+https://github.com/pikarpov-LANL/Sapsan.git"
  • [x] You should make "setup_gpu.py" part of setup.py. I think to do this, you can set the "extra_requires" part of the setuptools.setup. Then you would set, e.g., "pip install sapsan[gpu]" to get the GPU version.
• [x] However, I think you should not handle all of these requirements. Installing PyTorch can be tricky (look at all the options: http://pytorch.org/), so I would simply direct the user to install torch themselves, and then install your package.
• [x] The directory structure is too hierarchical. Instead of this:

  from sapsan.lib.backends.fake import FakeBackend
  from sapsan.lib.backends.mlflow import MLflowBackend
  from sapsan.lib.data import HDF5Dataset, EquidistantSampling, flatten
  from sapsan.lib.estimator import CNN3d, CNN3dConfig
  from sapsan.lib.experiments.evaluate import Evaluate
  from sapsan.lib.experiments.train import Train
  from sapsan.utils.plot import model_graph

  I would like to use something closer to this:

  from sapsan.backends import FakeBackend, MLflowBackend
  from sapsan.data import HDF5Dataset, EquidistantSampling, flatten
  from sapsan import CNN3d, CNN3dConfig, Evaluate, Train, model_graph

  You can use the __init__.py files to achieve something like this, even with a hierarchical file structure.

Documentation review:

• [x] I think it would be wise to have your example data automatically-downloaded by the Jupyter notebook, or some internal module (just like PyTorch will automatically download MNIST data for you), so one is not forced to clone the repo and work locally.
• [x] For the audience of people who do not normally do ML, I would make a Colab notebook that they can muck around with in-browser, without having to worry about dependency installation. If someone has to set it up on their own computer, I think you might lose half your potential users from the get-go.
  • You can go to this link: http://colab.research.google.com/github/pikarpov-LANL/Sapsan/blob/master/sapsan/examples/cnn_example.ipynb to get the github running. You should then add this link to your README. I would change your package and that notebook until you are able to completely run that notebook in Google colab - it is a good test of your software requirements being flexible enough.

Paper review

• [x] "Thus, it is not surprising that there is considerable skepticism against ML in physical sciences, with astrophysics being no exception."
  • [x] Personally, I don't think the skepticism is from a lack of easy-to-use software. If anything, ML toolkits are probably easier to use than most turbulence simulation software. I think the skepticism rightly stems from ML x Physics people not doing enough work to test the generalization of their models to configurations far outside of the test set, where the ML model will often fail, and this has produced skepticism in the community.
• [x] Paper is a bit light on references, and comparison to other work. See https://simdl.github.io/files/26.pdf for a list of other relevant references which you might want to include. Some of these references have code, and some do not.
• [x] Can you justify the use of MLFlow compared to other frameworks? I don't doubt it is a good option, but I am curious to hear why you chose it simply for my own interest and potential use.

Thanks! Let me know if you have any questions about these. Cheers, Miles

[pikarpov-LANL]

Overall:

• The target audience is both - people who do turbulence with and without ML. Sapsan is an attempt to structure and standardize ML implementation for turbulence modeling. This will make it much easier to read and implement the models from the papers. In Sapsan, the main focus is on the model structure itself, which is isolated in a separate script without the overhead. Also, I built Sapsan with Docker in mind for straightforward public releases of the models with the publications. In terms of jargon, I agree that I need to simplify the point of entry and I do plan to do so via tutorials.
  • As for the “model zoo”, it would be great! Actually, that’s the direction I want to build Sapsan in. For now, the models that I use and that I find interesting to try for my own research will be published through Sapsan. In addition, I am trying to engage the local LANL community to do the same through Sapsa, but open-sourcing anything at LANL is a big task for the researchers here. On my own, I do not have the capacity to add a significant amount of turbulence models to call Sapsan a “model zoo”; at least not yet.
• This is a very good point. Even though Sapsan is capable of doing pretty much anything related to ML, so are numerous other pipelines. Since Sapsan has been built with turbulence modeling in mind, I brought this focus forward in README and Wiki.

Code review:

• I generalized the requirements - updated compatibility to Catalyst 21.5 (there have been significant changes)
  • The only thing I didn’t touch is streamlit, which is hard-locked to a specific version. It is fast developing, yet not very well backwards compatible. While GUI is a fun way to demo models for presentation purposes, it won’t be used as much solely for research. Thus, I decided not to spend too much time on its maintenance right now with the latest streamlit versions (though it is fully operational).
• I removed the PyTorch installation and the separated GPU setup since it only different in the CUDA-enabled PyTorch installation. I agree that PyTorch should be installed separately. Wiki Installation has been updated.
• Hierarchy simplified.

Documentation review:

• I set up the google-colab notebook, which is identical to the cnn_example.ipynb, except the requirements installation. Now, folks can try out the jupyter notebook and the GUI demo both online.
• data will be updated to a better physical example as right now it is training on low-res velocity and predicting itself. What it will be is the working example from the papers I am writing at the moment - higher resolution JHTDB data to predict a closure turbulence model. The data will be updated once the paper will be in review (in a matter of weeks)

Paper review:

• I agree with your point - generalization testing is a big roadblock in our field. However, the argument I am trying to make is not around easy-to-use software, but a “black box” nature of ML. The latter is the most often used argument that I hear from the physics community. In my opinion, besides the convoluted nature of classic ML (no pun intended), accessibility is a major factor - not as much to the ML researchers building their own models, but to the physicists reading and evaluating the published work. It is especially relevant to the published work without released code. Through Sapsan I attempt to build a transparent pipeline, which includes physics-based analytic tools, and also bring physics-informed methods to a broader community (which are not trivial to build yourself straight from the papers without the code). While on my own I cannot solve generalizability, I can make improvements on the latter.
• References - no worries, I will send the updated version with more references in the joss github thread for this review, that’s no problem.
• MLflow has been suggested by my collaborator from the industry. Overall, I also considered kubeflow, but MLflow is much easier to track and analyze ensembles of simulations. One can plot various metrics and parameters over hundreds of runs with just a few clicks. Lastly, it is very straightforward to add more parameters to track if one needs to customize further.

[MilesCranmer]

Hi @pikarpov,

I sincerely apologize for the late followup. I have been on an internship where all the code writing is internal, and have been bad at going through my GitHub notifications, and this dropped off my radar. I am very sorry about that, and thanks for your patience.

The updated repo, documentation, and paper look significantly better. Fantastic work! This is a really great project that I am quite excited that you and your coauthors have created. I am very happy with how this work has turned out. The docs are really really nice, and I can clearly see how to implement different models. Thanks for looking into getting the colab working too - it's a time-consuming process to get that working, but it will be really useful for others in quickly trying out your package, so I think it will be worth it. I confirmed that it works as intended!

I think this will be a very useful repo for the community :) The README is very clear about the goals of the package, and I think they well-represent the framework. I think that updated focus will be very helpful in attracting others to work in this intersection of fields.

Cheers! Miles

[pikarpov-LANL]

Hi Miles,

Thank you for the kind words! A lot of work has gone into Sapsan. My collaborators and I appreciated both yours and Keaton's comments that helped improve the framework. We always welcome constructive criticism and suggestions!