Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggested improvements and fixes to JOSS paper #35

Closed
14 tasks done
matt-graham opened this issue May 22, 2024 · 5 comments
Closed
14 tasks done

Suggested improvements and fixes to JOSS paper #35

matt-graham opened this issue May 22, 2024 · 5 comments
Labels
documentation Improvements or additions to documentation

Comments

@matt-graham
Copy link
Contributor

matt-graham commented May 22, 2024

Raising as part of JOSS review openjournals/joss-reviews/issues/6713

Some assorted changes I would recommend making to the JOSS paper

  • Define acronyms in full on first usage (AI, API, QMC)
  • Rephrase first paragraph in Summary to make less disjointed
    • Perhaps something like 'Uncertainties are everywhere. Whether you are developing a new artificial intelligence system, running complex simulations or performing an experiment in a laboratory, uncertainties influence the system, and an approach is needed to understand how these uncertainties impact your results.' though maybe I'm not understanding the original text correctly. The connection of the 'And you need a ...' sentence to the preceding sentence is currently unclear as is what 'these' in the sentence refers to.
  • Rephrase second paragraph in Summary
    • Currently the last clause in the second sentence in paragraph doesn't make grammatical sense with respect to the first clause. I think perhaps something like 'Thanks to a clear Python API and an interactive web dashboard, [it / SimDec] makes uncertainty analysis accessible to everyone' would be better. I would potentially also scale back the claim that SimDec makes uncertainty analysis 'accessible to everyone' - which is quite a bold claim and difficult to verify, with something a bit more measured, for example 'SimDec offers an accessible approach to performing uncertainty analyses'.
  • Add bibliography entry and citation for 'AI Act'
    • Not clear what specifically is being referred to here. Remember the JOSS readership is international.
  • Add bibliography entry and citation for 'Better Regulation Guideline'
    • Again this needs a reference to make it clear what is being referred to.
  • Add bibliography entry and citation for Sobol' indices
  • Rephrase and expand 'decomposes the distribution of the output (target variable) by the multivariable scenarios'
    • This does not read right to me - the 'by' suggests the multivariable scenarios are the actor decomposing the distribution which I don't think is correct, though in general I think this sentence could do with expanding on to make it more understandable to a non-specialist audience (for example, what is a multivariable scenario here?)
  • Add more detail to caption of Figure 1 and make plot and table larger to improve readability
    • Currently it is difficult to understand what Figure 1 is showing as not much context is given. What is being plotted in the right panel and what is the table reporting? What 'input-output data' was used here. Aso the small size of the right panel makes it very difficult to read.
  • Correct typo 'through a Python packages available on PyPI' → 'through a Python package available on PyPI'
  • Make URL https://simdec.io pointing to web dashboard a hyperlink
  • Expand on what 'powerful methods' are used from SALib
  • Correct spacing around hyphen between SciPy citations and 'notable the...' or make an em-dash —
  • Add bibliography entry and citation for Panel (https://zenodo.org/doi/10.5281/zenodo.3706648)
  • Add state of the field comparison to other comparable packages
    • For example chaospy allows computing sensitivity indices as does SALib, and it looks like the package sensitivity might also be relevant.
@tupui
Copy link
Member

tupui commented May 24, 2024

Thank you for the feedbacks, we are tackling them.

Add state of the field comparison to other comparable packages
For example chaospy allows computing sensitivity indices as does SALib, and it looks like the package sensitivity might also be relevant.

We will try to rephrase some things to make this clearer. This package's main argument is visualisation. This is totally complementary to the packages that you mention and not at all to be compared with.

SALib, which I am a maintainer of, is the most complete package in Python for computing indices. Here we have one extra method to compute indices, but it's theoretically, and asymptotically, Sobol' indices. i.e. the same as what is in SALib and SciPy (recent version, I am a maintainer and the one who added it to SciPy with help from even Saltelli, Kucherenko and Owen). I am not talking about the others are not in the same league IMHO. OpenTURNS if you want to cite something else would be better.

The value proposition here is the visualisation method. It's not just a tool to show things, but a tool providing new insights and analysis.

@tupui tupui added the documentation Improvements or additions to documentation label May 24, 2024
@tupui
Copy link
Member

tupui commented May 30, 2024

@matt-graham we addressed your points, thanks again. What remains is providing a script to reproduce the figure itself. Moving this to the other issues.

@tupui tupui closed this as completed May 30, 2024
@matt-graham
Copy link
Contributor Author

Thanks @tupui, changes looks good. I've added a couple of small suggested changes to the updated text on #23.

@matt-graham
Copy link
Contributor Author

Sorry missed this part of your response previously:

SALib, which I am a maintainer of, is the most complete package in Python for computing indices. Here we have one extra method to compute indices, but it's theoretically, and asymptotically, Sobol' indices. i.e. the same as what is in SALib and SciPy (recent version, I am a maintainer and the one who added it to SciPy with help from even Saltelli, Kucherenko and Owen). I am not talking about the others are not in the same league IMHO. OpenTURNS if you want to cite something else would be better.

Being 'in the same league' is very subjective and I would say not of relevance here. There is an explicit requirement as part of the review checklist to check that the paper 'describe[s] how this software compares to other commonly-used packages' which I still think the current paper does not satisfy completely. I would also say only citing other packages for which you are a developer is not ideal. The examples I gave were not meant to be prescriptive and agree that OpenTURNS looks like it would be another relevant package to compare to, though I also think chaospy falls in this category. Note that what you said in

The value proposition here is the visualisation method. It's not just a tool to show things, but a tool providing new insights and analysis.

would itself be a useful comparison point to put in paper to make clear what SimDec brings over existing available packages.

@tupui
Copy link
Member

tupui commented May 30, 2024

Being 'in the same league' is very subjective and I would say not of relevance here

Not relevant for this paper yes. Still, SALib and SciPy are the references in Python. Like sensobol would be a reference in R. This is not an opinion but a fact. See https://www.sciencedirect.com/science/article/pii/S1364815224000380

I would also say only citing other packages for which you are a developer is not ideal.

Well, I contributed to almost all of them in the Python space. Either by creating issues, talking privately to maintainers, or at workshops. This is a small space and we all know each other. Both on the dev side and the research side.

OpenTURNS, I made my PhD with it and interacted with the devs and collaborated on a few projects with them (see my Batman paper in JOSS for a quick glance.)

chaospy is only providing Sobol' indices as an output from the Polynomial Chaos decomposition. i.e. not at all the focus of the project. It's similar to scikit-learn which proposes to get feature importance with a Random Forest.

would itself be a useful comparison point to put in paper to make clear what SimDec brings over existing available packages.

The text now reads:

Traditional methods to analyse the uncertainties focus on quantitative methods
to compare the importance of factors, there is a large body of literature and
the field is known as: Sensitivity Analysis
[...]
Simulation Decomposition or SimDec moves the field of SA forward by supplementing the computation of sensitivity indices with the visualization of the type of interactions involved,
[...]
In short, SimDec is a hybrid uncertainty-sensitivity analysis approach
that reveals the critical behavior of a computational model or an empirical
dataset.

Without rewriting an article about SimDec, I don't see what is missing here to say that the visualisation is the new thing. No other package has it because none of them is focused on visualisation. I don't see what it would bring to the discussion to add the list of packages about SA and say they don't do it. It's out of their scope, nothing more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants