desreslab / linfa Goto Github PK

I've successfully run the trivial, highdim, and adaan examples, and have a few questions and comments:

• I discovered how to run fewer iterations by looking at the source code and .github/workflows/test_publish_pypi.yml, but it might be helpful to point out somewhere that this can be controlled by setting the it environment variable.
• The adaan example appeared to run for more than the specified number of iterations? For example, with it=100 it looked to run 370 iterations, and with it=1000 it looked to run 1270 iterations. Have I misinterpreted the output messages?
• When the number of iterations is less than exp.save_interval, no outputs are saved. It could be useful to always save outputs at the end of the experiment, in addition to doing so at every save interval?

Easier way to call linfa from command line with -m option to

• execute the test cases
• delete files for test cases
• post-process results

JOSS guidelines require reproducibility of the results in the paper. Could you please elaborate on this, and also can you indicate this matter in the paper?

JOSS review point: "If the paper contains original results, results are entirely reproducible by reviewers."

The Numerical Examples section of the documentation introduces a range of mathematical models, and shows various output plots generated by LINFA.

• To help the reader understand how each model is implemented in LINFA, it might be helpful to either (a) link to the Python source file for each example; or (b) use the literalinclude directive to include relevant chunks of code in the documentation itself.

• Does the log-loss for the Three-element Windkessel Model get worse as the number of iterations increases? If so, should this page include some comments that point this out and discuss the implications for the obtained fit?

• The README.md links to two very helpful notebook tutorials (2D and 3D ballistic models). These notebooks demonstrate how to perform inference tasks and highlight some of LINFA's key features. I found them very helpful in trying to understand how to run LINFA with my own model (as opposed to running one of the models in the test suite) — I felt that these notebooks were the missing link I was looking for.

  It would be great to have some of this information included in the online documentation; possibly as a separate page under "Numerical examples", or anywhere else that the authors feel is suitable. I did a search and found that Sphinx can render .ipynb files as HTML pages with the nbsphinx extension.

links to tables and figures work locally but do not work on readthedocs

Joss's editorialbot reports at openjournals/joss-reviews#6309

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- None

MISSING DOIs

- 10.1016/b978-0-08-051581-6.50057-x may be a valid DOI for title: Stochastic relaxation, Gibbs distributions, and the Bayesian restoration of images
- 10.2172/4390578 may be a valid DOI for title: Equation of state calculations by fast computing machines
- 10.1093/oso/9780198509936.003.0015 may be a valid DOI for title: Monte Carlo sampling methods using Markov chains and their applications
- 10.21236/ada208388 may be a valid DOI for title: Sampling-based approaches to calculating marginal densities
- 10.1561/9781601981851 may be a valid DOI for title: Graphical models, exponential families, and variational inference
- 10.1109/tpami.2020.2992934 may be a valid DOI for title: Normalizing flows: An introduction and review of current methods
- 10.1016/j.jcp.2022.111454 may be a valid DOI for title: Variational inference with NoFAS: Normalizing flow with adaptive surrogate for computationally expensive models
- 10.1615/int.j.uncertaintyquantification.2022043110 may be a valid DOI for title: AdaAnn: Adaptive Annealing Scheduler for Probability Density Approximation
- 10.1016/s0951-8320(02)00229-6 may be a valid DOI for title: Theorems and examples on high dimensional model representation
- 10.1016/0041-5553(67)90144-9 may be a valid DOI for title: On the distribution of points in a cube and the approximate evaluation of integrals

INVALID DOIs

- None

Please check for each of them that the proposed DOI is OK, and if so add it to your bib file.

After running some examples (#13) I also have a few questions and comments about plotting the results:

• The linfa.plot_res module is mentioned in README.md, but not anywhere in the online documentation (unless I've missed it). I think it's plausible that a user might look at the online documentation, but not the GitHub repository, so perhaps this module could be included in the online documentation?

• I discovered that the folder name must end with a trailing slash, otherwise it raises an error:

  python -m linfa.plot_res -n trivial -i 25000 -f results
  Log file not found: resultstrivial/log.txt

  Line 162 of plot_res.py concatenates the folder and experiment names. Please consider using os.path.join() or pathlib.Path to construct file names.

no access to the test cases from the library when it is installed from pypi.

Are there any other commonly-used packages? Either way, please clearly state them in the paper (and in the readme if you wish).

.idea folder sohuld be removed to improve cleanliness of the code. It is already in the .gitignore

Hello, I am reviewing the paper of this repository.

Please provide clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

For 2) and 3), I believe adding short sentences wiith directives to the readme would be enough, and for 1), please check https://mozillascience.github.io/working-open-workshop/contributing/ .

Hi @daneschi, I've started my JOSS review and encountered the following issue:

python3 -m venv venv
. venv/bin/activate
pip install linfa-vi
python -m unittest linfa.linfa_test_suite.trivial_example
# ModuleNotFoundError: No module named 'linfa.models'

I can see that linfa.models exists in the repository itself, and the tests run successfully on GitHub CI. The cause appears to be that linfa.models isn't being included in the wheel or sdist. However, because the repository has a "flat layout", linfa.models can be imported from the source directory when running the CI tests.

The packages can be fixed by making the following changes to pyproject.toml:

[tool.setuptools]
packages = ["linfa", "linfa.models", "linfa.tests"]

or. to avoid having to manually list each sub-package:

[tool.setuptools]
packages.find.include = ["linfa*"]

If you would like to ensure that the CI tests are being run against the built package, you could switch to a "src layout" (move linfa to src/linfa), and install the built package before running the tests. It's a little fiddlier, and a tool such as tox or nox can be helpful for this kind of thing. This is just a thought, I'm not suggesting you should do this as part of the JOSS review.

add step-by-step tutorial on physics-based model

/opt/hostedtoolcache/Python/3.8.17/x64/lib/python3.8/site-packages/torch/functional.py:504: UserWarning: torch.meshgrid: in an upcoming release, it will be required to pass the indexing argument. (Triggered internally at ../aten/src/ATen/native/TensorShape.cpp:3190.)
return _VF.meshgrid(tensors, **kwargs) # type: ignore[attr-defined]