openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
725 stars 38 forks source link

[REVIEW]: DuneCopasi: A multi-compartment reaction-diffusion simulator for systems biology #6836

Open editorialbot opened 5 months ago

editorialbot commented 5 months ago

Submitting author: !--author-handle-->@SoilRos<!--end-author-handle-- (Santiago Ospina De Los Ríos) Repository: https://gitlab.dune-project.org/copasi/dune-copasi Branch with paper.md (empty if default branch): master Version: v2.0.1 Editor: !--editor-->@prashjha<!--end-editor-- Reviewers: @ayush9pandey, @ajbaird Archive: Pending

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/f5287f008aa398e96ebcdce8ef51b2c9"><img src="https://joss.theoj.org/papers/f5287f008aa398e96ebcdce8ef51b2c9/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/f5287f008aa398e96ebcdce8ef51b2c9/status.svg)](https://joss.theoj.org/papers/f5287f008aa398e96ebcdce8ef51b2c9)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@ayush9pandey & @ajbaird, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @prashjha know.

Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest

Checklists

📝 Checklist for @ayush9pandey

📝 Checklist for @ajbaird

editorialbot commented 5 months ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf
editorialbot commented 5 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.10 s (2082.8 files/s, 279594.7 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
C/C++ Header                     55           1273            873           6395
SVG                               4              3              1           4554
Markdown                         45           1212              0           4251
C++                              16            331             93           1827
JavaScript                       11             97             27           1462
JSON                              8              0              0            977
CMake                            31            151             57            762
INI                               9            102              0            404
Bourne Again Shell                6            135            110            401
YAML                              4             34              5            392
Dockerfile                        3             47             26            242
Python                            3             45              9            234
TeX                               1             15              0            233
CSS                               3             21             33             91
GLSL                              1              8              6             16
Bourne Shell                      1              9              4             14
--------------------------------------------------------------------------------
SUM:                            201           3483           1244          22255
--------------------------------------------------------------------------------

Commit count by author:

   689  Santiago Ospina De Los Rios
   340  Santiago Ospina De Los Ríos
    50  Liam Keegan
    13  Santiago Ospina
     3  Joseph Holten
     3  dependabot[bot]
     1  Dylan Vermoortele
     1  Johanna Biereder
     1  lkeegan
editorialbot commented 5 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1007/s00607-008-0003-x is OK
- 10.1016/j.camwa.2020.06.007 is OK
- 10.1002/nme.2579 is OK
- 10.1016/j.csbj.2014.05.007 is OK
- 10.1093/bioinformatics/btg015 is OK
- 10.1016/s0006-3495(97)78146-3 is OK
- 10.1093/bioinformatics/btt772 is OK
- 10.1515/jnma-2023-0089 is OK
- 10.1515/jnum-2012-0013 is OK
- 10.1016/j.cpc.2015.02.008 is OK
- 10.5281/zenodo.10246531 is OK
- 10.7554/eLife.78540 is OK
- 10.21105/joss.05735 is OK
- 10.18419/opus-3620 is OK
- 10.1038/ncomms11437 is OK

MISSING DOIs

- No DOI given, and none found for title: Generic implementation of finite element methods i...

INVALID DOIs

- None
editorialbot commented 5 months ago

Paper file info:

📄 Wordcount for paper.md is 1370

✅ The paper includes a Statement of need section

editorialbot commented 5 months ago

License info:

🟡 License found: Other (Check here for OSI approval)

editorialbot commented 5 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

ayush9pandey commented 5 months ago

Review checklist for @ayush9pandey

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

ajbaird commented 4 months ago

Review checklist for @ajbaird

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

prashjha commented 3 months ago

Hello @ayush9pandey and @ajbaird, based on your checklists, I see very little progress. This is a gentle reminder, and I would appreciate it if you could provide an update on the progress of the review. Thank you for your efforts! :)

ayush9pandey commented 3 months ago

Thanks for the reminder @prashjha. Unfortunately the review progress was slow because I needed access to create issues on the original repository -- it is restricted by the DUNE organization. I got the access on Jul 22, and am planning to complete my review soon.

SoilRos commented 3 months ago

Hi, @ajbaird and @ayush9pandey, if you have any issues with the registration on the DUNE GitLab or with the repository itself, please let me know here. @ajbaird the registration process is simple but is not automatic, you need to send us an email to gitlab-admin at dune-project.org in order to create an account with the correct name and credentials. Alternatively, you both can just open issues in the repository by sending an email to dune-copasi at dune-project.org. I am looking forward for the reviewing process, thank you!

ajbaird commented 3 months ago

Hi all! should be able to get to this this week, sorry was out on vacation/out of office the past 11 days.

ayush9pandey commented 3 months ago

@prashjha Dune-Copasi is a solver for reaction-diffusion systems and supports multiple compartments in the models. The package provides GUI, API, and CLI interfaces to run the package, depending on the user preferences, and on what the user's OS might allow/be compatible with. The package implements the finite element method and provides various user friendly ways to input initial conditions in reaction-diffusion system equations. Thus, the development of this package is important. A concern that I have right now is the presentation of the code, usage, and the paper are geared for specialists in the area of authors' research while even other researchers in the broader area of computational biology might find it hard to understand or use the package. I have provided some comments below on the software and the paper that might give ideas for improvement of the presentation.

Software related issues that the authors may consider in improving the development and delivery of the package:

  1. Installation and tutorial use of dune-copasi is not straightforward (at least to me). Installing with package manager is only supported for Ubuntu distributions, installation from source requires plenty of dependencies that are not automatically managed/installed. A docker installation is recommended but I ran into an error when following that approach (https://gitlab.dune-project.org/copasi/dune-copasi/-/issues/93). Overall, it seems pretty challenging for Windows users to set up while it might be easier for Linux. If there are specific steps that are most recommended for each OS, then I would recommend outlining these before giving the options of going different routes for the installation.
  2. I was able to run the GUI that is provided for Windows OS but could not test the API and CLI use, due to the difficulties with the installation. For the GUI, I followed the tutorial on the documentation (https://dune-copasi.netlify.app/docs/use_gui), however, the tutorial is not detailed enough for me to learn the Spatial Model Editor. What's something that Dune-Copasi can do within Spatial Model Editor? I would have expected to see that tutorial here. The README of the package is another place where some basic features can be demonstrated. However, right now the package installation/use link is broken and other than a broad overview there are no examples given. Related issue: https://gitlab.dune-project.org/copasi/dune-copasi/-/issues/92
  3. I noticed that there is also a Github repository (https://github.com/dune-copasi/dune-copasi) in addition to the git-lab repository for the package. Not sure what's the recommended one to follow for all updates.
  4. Can the authors point to how to run their automated tests? (https://gitlab.dune-project.org/copasi/dune-copasi/-/issues/94)
  5. I could not find community guidelines and contributing instructions.

Comments on the paper that the authors may consider:

  1. Why does the package have "COPASI" in its name? This could cause confusion to new users who've used (arguably, the more well known) computational biology GUI and package, also called Copasi before (https://copasi.org/).
  2. Summary does not align with JOSS instructions: The summary should have "a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience". In my opinion, the language right now is a bit dense for a non-specialist to follow.
  3. A section or summary of background is better to include before presenting the "Model problem".
  4. In the model problem, what are the compartments and "the problem". I think these need to be defined before giving the mathematical formulation.
  5. Capitalize D in *dirichlet?
  6. Towards the end of the first page (lines 28-30), it might be better to give examples of code or features of package that implement the mathematically defined constructs.
  7. Line 34 citation format is probably incorrect with just the year. The authors should make sure that all citations are correctly formatted as required by JOSS.
  8. The Spatial Model Editor is referred in Line 35 without reference or description of what it might be or how it relates to dune-copasi.
  9. In caption of Figure 1, towards the end, is the experimental data from Wehling et al. 2022? Worth mentioning if that's the case.
  10. In Figure 2, it's not clear to me which parts did DuneCopasi solve? Worth adding some software specific details to the pipeline that is presented here.
  11. The statement of need could probably be moved before the previous two sections because it sets up the motivation.
  12. Is there an easy comparison for what VCell can / cannot do as compared to DuneCopasi?
SoilRos commented 3 months ago

Hey @ayush9pandey, thanks for the thorough review! Since there 17 different points with different degree of complexity and associated work, I will try to answer them in order of increasing difficulty with a priority on the software side so that you can continue your review without much disruption.

SoilRos commented 3 months ago
  1. I could not find community guidelines and contributing instructions.

Every page of the documentation website contains a section named "Community" on the bottom of the page. In particular, it contains a link to the participation guidelines. This page contains the instructions to submit reports on typos or misleading manual instructions, bug reports, and feature requests. This page also points to our contribution guidelines in the root directory on the repository together with our code of conduct. If you think there is another place that would be more suitable for this guidelines, please let me know.

ayush9pandey commented 3 months ago

Missed that, my apologies!

On Mon, Aug 12, 2024, 04:25 Santiago Ospina De Los Ríos < @.***> wrote:

  1. I could not find community guidelines and contributing instructions.

Every page of the documentation website https://dune-copasi.netlify.app/ contains a section named "Community" on the bottom of the page. In particular, it contains a link to the participation guidelines https://dune-copasi.netlify.app/participate. This page contains the instructions to submit reports on typos or misleading manual instructions, bug reports, and feature requests. This page also points to our contribution guidelines https://gitlab.dune-project.org/copasi/dune-copasi/-/blob/master/CONTRIBUTING.md in the root directory on the repository together with our code of conduct https://gitlab.dune-project.org/copasi/dune-copasi/-/blob/master/CODE_OF_CONDUCT.md. If you think there is another place that would be more suitable for this guidelines, please let my know.

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/6836#issuecomment-2283710640, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6DDO5UYEGX3VPUGHQTLXTZRCLS5AVCNFSM6AAAAABIYIMETCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOBTG4YTANRUGA . You are receiving this because you were mentioned.Message ID: @.***>

SoilRos commented 3 months ago
  1. I noticed that there is also a Github repository (https://github.com/dune-copasi/dune-copasi) in addition to the git-lab repository for the package. Not sure what's the recommended one to follow for all updates.

Both are valid repositories to follow the updates. Technically, the GitHub repository is a mirror of the GitLab repository. I just updated the description of the GitHub repository to make this more clear. On the other hand, the development of the project happens in the DUNE GitLab, and this is reflected in the participation landing page and the contribution guidelines. If there is another way I could make this distinction more clear, I would be happy to hear suggestions :)

SoilRos commented 3 months ago
  1. Installation and tutorial use of dune-copasi is not straightforward (at least to me).

This is a fair point. Installation is not easy due the many requirements, that's why we have several installation manuals. But choosing the most suitable installation medium is probably only clear after reading all of the installation manuals. Do you think that adding an installation introduction on this page would help to solve this issue?

SoilRos commented 3 months ago

For 2. I would like to break it down into different issues:

I was able to run the GUI that is provided for Windows OS but could not test the API and CLI use, due to the difficulties with the installation.

I believe that your difficulty is with the usage, not with the installation. I believe that this can be addressed by solving https://gitlab.dune-project.org/copasi/dune-copasi/-/issues/93.

For the GUI, I followed the tutorial on the documentation (https://dune-copasi.netlify.app/docs/use_gui), however, the tutorial is not detailed enough for me to learn the Spatial Model Editor. What's something that Dune-Copasi can do within Spatial Model Editor? I would have expected to see that tutorial here.

The issue is that both the DuneCopasi and Spatial Model Editor are disjoint projects made by different people within the same institution. The Spatial Model Editor uses DuneCopasi to perform its simulation as described in the paper. However, if we give examples of usage on this documentation website (owned by DuneCopasi), it is likely to get outdated due to out of sync changes on the two repositories. This is why the documentation explicitly points the user to the documentation of the Spatial Model Editor instead of providing a fully working example that may be outdated by the time of reading. What I think could help to avoid this confusion is to make this recommendation more prominent in the documentation by using a admonition. @ayush9pandey, do you think this help to mitigate this problem?

The README of the package is another place where some basic features can be demonstrated. However, right now the package installation/use link is broken and other than a broad overview there are no examples given. Related issue: https://gitlab.dune-project.org/copasi/dune-copasi/-/issues/92

This particular issue with the link is already solved by https://gitlab.dune-project.org/copasi/dune-copasi/-/merge_requests/211. Is this sufficient or do you think that the README still needs more work?

SoilRos commented 3 months ago

Regarding the points 1. and 2. on the software side, I have the impression that the point highlighted in https://gitlab.dune-project.org/copasi/dune-copasi/-/issues/93 stopped you from actually interacting with the software. This is a usage error because meaningless input will naturally throw an error (the opposite behavior would be surprising IMO). This is an mistake in our documentation, I will try to address this with a more clear documentation on the usage part of the installation documentation because this sees to have caused the confusion. But while I fix this part of the documentation, I would like to highlight that there is a tutorial section and a documentation of the CLI that provides the necessary information to give a meaningful input.

Please also note that the main objective of the online documentation is to explain the CLI usage which must be the entry point to use DuneCopasi. The GUI is covered by the Spacial Model Editor documentation whereas the API usage is so specialized and rarely used that we left it as an offline class documentation as described in here.

ayush9pandey commented 3 months ago

Do you think that adding an installation introduction on this page would help to solve this issue?

Yes.

What I think could help to avoid this confusion is to make this recommendation more prominent in the documentation by using a admonition. @ayush9pandey, do you think this help to mitigate this problem?

Yes, that would be helpful.

This particular issue with the link is already solved by https://gitlab.dune-project.org/copasi/dune-copasi/-/merge_requests/211. Is this sufficient or do you think that the README still needs more work?

I noticed that on your git-lab, many of the common items of a readme are provided on the right side bar -- so that's great! I think readme files can include some other information too, but that's obviously not a requirement, just my opinion. I commonly see that readme files have a combination of the following in various packages: a banner about the project, some important quick links to accompany the main features, a minimal working example, a citation recommendation, etc. You can choose the appropriate level of detail that you'd want for your package.

This is a usage error because meaningless input will naturally throw an error (the opposite behavior would be surprising IMO). This is an mistake in our documentation, I will try to address this with a more clear documentation on the usage part of the installation documentation because this sees to have caused the confusion. But while I fix this part of the documentation, I would like to highlight that there is a tutorial section and a documentation of the CLI that provides the necessary information to give a meaningful input.

Thanks for working on fixing the usage documentation -- that would be very helpful. I'll try out the CLI documentation that you pointed out.

Please also note that the main objective of the online documentation is to explain the CLI usage which must be the entry point to use DuneCopasi.

Got it. This might be highlighted in the paper if you were to point to specific CLI features that are shown in the illustrative figures.

ajbaird commented 3 months ago

Hi @prashjha still verifying the build from source on my wsl install but my partial review edits are included here:

I'll have more but it looks like @ayush9pandey did an excellent job reviewing the paper and we share many of the issues he outlines in the paper. I want to confirm building form source, I'll add edits if there are issues there!

Thanks and sorry for the delay!

Austin

SoilRos commented 3 months ago

Hi @ajbaird, thanks for the comments! I will be working on fixing these issues. For now:

  • the curl command doesn't work (cannot resolve host) to install using a linux package manager

There was a double curl on the command. Probably some copy paste error from my side. It should be fixed now. Thanks for pointing this out.

  • the gitlab project cannot be cloned, this needs to be documented

Can you expand on this? I never encountered this problem before.

SoilRos commented 3 months ago
  • the doxygen documentation on API should be provided on the website, I don't see anyway to reference api, not user build required

This is something that I always wanted and indeed have tried before (see https://gitlab.dune-project.org/copasi/dune-copasi/-/merge_requests/73), however, it turned out to have so much maintenance cost for me and with so little benefit that I decided against this and ended up reverting it. As a trade off I wrote a short explanation on how to locally build the documentation. If this happens to be a deal breaker, I would rather strip the API altogether from the documentation and the paper.

SoilRos commented 3 months ago

I addressed the following issues, and they should be already visible in the "next" version of the documentation (This will become the next default when all things are addressed).

  • options file in the source install doesn't reference where you are in terms of the project, directories should always be specified during build instructions
  • the cli documentation should provide a link to the binary, or link to the build process to build

@ajbaird Please let me know in case you don't think they were correctly resolved.

ajbaird commented 3 months ago
  • the doxygen documentation on API should be provided on the website, I don't see anyway to reference api, not user build required

This is something that I always wanted and indeed have tried before (see https://gitlab.dune-project.org/copasi/dune-copasi/-/merge_requests/73), however, it turned out to have so much maintenance cost for me and with so little benefit that I decided against this and ended up reverting it. As a trade off I wrote a short explanation on how to locally build the documentation. If this happens to be a deal breaker, I would rather strip the API altogether from the documentation and the paper.

I totally understand it can be hard to get this right. I'm not going to make it an issue big enough to not publish this paper but I strongly recommend hosting your api documentation on your website. You can generate straight html files with doxygen (there is a flag for it). Then you can host all the html, in some directory called /docs/ or something on your website, then it should all be able to be rendered you just need a link to it on your webpage. I'd recommend at some point in the near future you add it, its really critical for developers to have access to it and not have to generate it themselves.

here is a good reference for you https://github.com/BioGearsEngine/core/blob/trunk/share/doc/doxygen/doxyfile.in this is how we configure our doxygen output.

SoilRos commented 3 months ago

@ajbaird Sorry if my answer came across as sharp, it was not my intention! The problem has never been how to generate the doxygen documentation but rather on how to serve it in the website. I am not a web developer so having to interact with the internals of docusaurus (our website generator) to do this is very slow and painful. I have tried this multiple times (!) because I do agree with you, but after an unjustifiable amount of hours I always ended up frustrated and not having a proper solution.

However, I gave it another try this week and now I am quite happy with the result. Doxygen docs will now be published for every release and latest changes, and the website nicely embeds them automatically within the docs :-)

ajbaird commented 3 months ago

@SoilRos oh I didn't interpret the answer as sharp at all! Glad you got it working!

ajbaird commented 3 months ago

@prashjha I'm happy with the changes made to my comments. The doxygen addition is really nice for the community. I've completed the checklist and my bulleted edits have been addressed.

SoilRos commented 3 months ago

I think I fixed most (if not all) of the issues with the documentation. In particular, I added some examples that are automatically tested. I will be working on the paper next week.

Regarding the GUI, I would like to highlight that these are two different software. I decided to put the SME in the documentation because is a very low bar for someone who is into systems biology and doesn't know how to use the terminal. This presentation maybe comes with the disadvantage that it gives the impression that Spatial Model Editor (SME) is "just" a GUI for DuneCopasi. It is not. The SME is the one that does the heavy lifting with the SBML whereas DuneCopasi worries about the simulation part making sure that its internal are compatible with the SME/SBML needs. Or in other words, the SME "just" contains an instance of DuneCopasi to run its simulations. If you have suggestions on how to make this more clear in the website, I will gladly incorporate them.

Regarding what COPASI stands for in this project, it's already somewhat answered in our Community page. I will add a comment about this in the paper. Perhaps a bit more in detail: The reality is that when we started the project we thought that there could be more parts of the code to reuse from COPASI, but later we realized that it was not worth to bring the "heavy" COPASI internals into DUNE: The COPASI reaction mechanisms were easily made replaced with a math parser whereas the PDE parts did need the heavy lifting of a finite element framework like DUNE. That was too late to change the name of this engine simulator, but we thought that this was also fine given its origin. On the other hand, to read models made by COPASI in the SME it turned out to be useful to directly reach out for some of the COPASI internals, so the names got a bit swapped out in the end.

prashjha commented 3 months ago

@prashjha I'm happy with the changes made to my comments. The doxygen addition is really nice for the community. I've completed the checklist and my bulleted edits have been addressed.

Great! Thank you!

SoilRos commented 1 month ago

@editorialbot generate pdf

editorialbot commented 1 month ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

SoilRos commented 1 month ago

I am sorry for the very long delay. We had to coordinate among the authors some of the responses and modifications. Unfortunately, this collided with school and university holidays here in Germany.

@ayush9pandey I hope the following addresses most if not all of the concerns raised in https://github.com/openjournals/joss-reviews/issues/6836#issuecomment-2283028040:

  1. Why does the package have "COPASI" in its name? This could cause confusion to new users who've used (arguably, the more well known) computational biology GUI and package, also called Copasi before (https://copasi.org/).

The title of the software project was chosen because It originated in members of the Copasi team (responsible for the COPASI software) and the DUNE team combining their expertise to create tools for spatial PDE-based modeling in systems biology. The software described in this manuscript is one of the major building blocks of this effort. We have made this disclosure explicit in the paper.

  1. Summary does not align with JOSS instructions: The summary should have "a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience". In my opinion, the language right now is a bit dense for a non-specialist to follow.

  2. A section or summary of background is better to include before presenting the "Model problem".

  3. In the model problem, what are the compartments and "the problem". I think these need to be defined before giving the mathematical formulation.

  4. Capitalize D in *dirichlet?

  5. The statement of need could probably be moved before the previous two sections because it sets up the motivation.

I restructured the text to address these requests. In particular, I moved two paragraphs from the Statement of Need into a Background section before the Math Model section. I also stripped technical terms from the summary and moved them into the Math Model section. Furthermore, I expanded the Math Model section to give a more explicit definition of each term, this is longer (I hope this is fine) but much more understandable even if the reader is not a math expert.

  1. Towards the end of the first page (lines 28-30), it might be better to give examples of code or features of package that implement the mathematically defined constructs.

The problem is that particular biological models are typically very lengthy. I wanted to avoid this by referring to Eliaš & Clairambault (2014) who present several models in a very detailed manner and can all be implemented in our software. Even a "simple" Cardiac Electrophysiology model would occupy several paragraphs of the paper. On the other hand, I extended the explanation of the math model to make absolutely clear that every parameter of the model is customizable.

  1. Line 34 citation format is probably incorrect with just the year. The authors should make sure that all citations are correctly formatted as required by JOSS.

The authors are part of the sentence. As of today, this is a format advertised by the JOSS guidelines.

  1. In caption of Figure 1, towards the end, is the experimental data from Wehling et al. 2022? Worth mentioning if that's the case.
  2. In Figure 2, it's not clear to me which parts did DuneCopasi solve? Worth adding some software specific details to the pipeline that is presented here.

Both figures have been corrected.

  1. Is there an easy comparison for what VCell can / cannot do as compared to DuneCopasi?

We do not have direct comparisons so far, that's why we only describe the differences from the technical specifications of the two.

SoilRos commented 1 month ago

@editorialbot generate pdf

editorialbot commented 1 month ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

ayush9pandey commented 1 month ago

Thank you so much for updating the paper! I am mostly satisfied with all changes to the paper. The updated Figure 2 is great at giving the overview of the software, it might make sense to make that a headline (by showing this figure earlier) and then show the predictive power in Figure 1.

For the citation format, maybe the editor can chime in, but these don't look consistent to me:

Image 1: image

Image 2: image

(and there are similar others with just the year)

I'd have expected image 2 to also have image 1 style citation with blue colored author names hyperlinked to the citation, not just the year?

I am planning to review the software by running some examples this weekend to complete my review.

prashjha commented 1 month ago

@ayush9pandey, I agree that citations need to be more consistent, which @SoilRos can fix quickly.

prashjha commented 1 month ago

@ajbaird, many thanks for your reviews and for letting us know the recommendation! Best wishes, Prashant

ajbaird commented 1 month ago

@prashjha thanks again! Was happy to review and see the edits, I think in the end it came out really really well! A great paper to be proud of!

SoilRos commented 1 month ago

@prashjha, @ayush9pandey, @ajbaird thank you very much for your feedback!

I agree that citations need to be more consistent, which @SoilRos can fix quickly.

I still believe that in-text-citation is correct as this format is common in many citation styles (e.g. APA), in particular when the author's name is part of the narrative. However, I agree that it looks strange due the hyperlink not highlighting the author name as well. I believe this could be corrected somehow on the pandoc settings. In any case, I re-formulated the text containing those in-text-citations to avoid this problem.

The updated Figure 2 is great at giving the overview of the software, it might make sense to make that a headline (by showing this figure earlier) and then show the predictive power in Figure 1.

Thanks, that makes sense. I moved it into the "Capabilities" section and made a weak reference to it on the last sentence of the section. This way figures are presented in the same order they are presented.

SoilRos commented 1 month ago

@editorialbot generate pdf

editorialbot commented 1 month ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

ayush9pandey commented 6 days ago

Apologies for the long delay in finishing my review. The paper looks good to me. I am reviewing the package functionality now. The package documentation is much improved and the variety of installation options are great!

Unfortunately, I still cannot get it to work other than through the GUI on my Windows. I feel that most instructions are still geared towards MacOS and Linux users. I tried installing through Docker but Docker Desktop shows errors when running (error with container init), and on the terminal I have other errors.

But since it works with the GUI, I am willing to mark the functionality checks of the JOSS review as it's most likely just my inability to run everything on Windows. At the same time, I have given quite a few hours and am unwilling to spend more time testing and reviewing all of the functionality that's not on the GUI.

As I said earlier, the package development is great and the application is well motivated.

ajbaird commented 5 days ago

Hi @ayush9pandey I was able to test on my linux VM and everything worked great!

SoilRos commented 5 days ago

@ayush9pandey, did you happen to try the web browser CLI? There, I can run models even on my phone. Or did you have problems with it too?

Could you share the problems that you had with docker? Were they related to Docker itself or with the image provided in the registry? In either case, this could help us have a better installation/usage guide. I do not use windows too much so I cannot foresee the problems one can have with docker. While "it should work", maybe we can give more specific instructions for Windows users based on your feedback.

ayush9pandey commented 5 days ago

Yes, the web version and the GUI work fine and that's how I was able to check the functionality of the tool and complete my review. Thank you for providing these user-friendly options!

I wanted to test things locally but that seemed to not work for me, which is probably due to some error that's my fault. I'll see if I can raise the issues on the Gitlab project rather than pinging everyone on this thread / holding back this review.

prashjha commented 3 days ago

Hi @ayush9pandey, thank you for the thorough review!!

prashjha commented 3 days ago

Hi @SoilRos, I am reading the draft and will let you know if I have any suggestions.

Meanwhile, could you please archive (if not done already) the release using zenodo and provide the archive reference so that I can associate it with your JOSS submission? Also, please ensure that the zenodo archive's title matches the title of this JOSS article.

If you have updated a version of your code, let me know, and I can update it here.

prashjha commented 3 days ago

@editorialbot generate pdf