openjournals / joss-reviews

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

[REVIEW]: dfba: Software for efficient simulation of dynamic flux-balance analysis models in Python #2342

Closed whedon closed 4 years ago

whedon commented 4 years ago

Submitting author: @davidtourigny (David Tourigny) Repository: https://gitlab.com/davidtourigny/dynamic-fba Version: v0.1.8 Editor: @marcosvital Reviewer: @jdbrunner, @pstjohn, @synchon Archive: 10.5281/zenodo.4009224

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

status

Status badge code:

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

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

@jdbrunner & @pstjohn & @synchon, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

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

Please try and complete your review in the next six weeks

Review checklist for @jdbrunner

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @pstjohn

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @synchon

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @jdbrunner, @pstjohn, @synchon it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

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

@whedon commands

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

@whedon generate pdf
whedon commented 4 years ago
Reference check summary:

OK DOIs

- 10.1038/nbt1094-994 is OK
- 10.1038/nbt.1614 is OK
- 10.1016/S0006-3495(02)73903-9 is OK
- 10.1007/s00211-015-0760-3 is OK
- 10.1186/s12859-014-0409-8 is OK
- 10.1101/700112 is OK
- 10.1145/1089014.1089020 is OK
- 10.1186/1752-0509-7-74 is OK
- 10.21105/joss.00139 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

marcosvital commented 4 years ago

Dear @davidtourigny, @Midnighter and @carrascomj: your manuscript will be reviewed in this issue, and you can reply any comments and suggestions that the reviewers might address right here. Please take a last look at the manuscript proof above, since this is the version they will use to start the review.

davidtourigny commented 4 years ago

Thanks, the proof looks OK to me but welcome suggested improvements from the reviewers.

marcosvital commented 4 years ago

@jdbrunner, @pstjohn, @synchon, thank you all for for accepting review this submission for JOSS. Even if you are not starting the review right now, please accept the invite, as it has an expiration date (there is a link under Reviewer instructions & questions and you should also get an email notification). Furthermore, please check the instructions and checklists above, and let me know if you need any assistance.

You can also tag @davidtourigny, @Midnighter and @carrascomj if you need to ask specific questions about the submission.

pstjohn commented 4 years ago

This is a great contribution, and I think the paper is well-written. There's certainly a need for a high-performance, easy-to-use DFBA implementation, and I think this package goes a long way in solving that need. Writing something like this is not easy, it takes someone with knowledge / background in dynamic systems, software engineering (including writing compiled python extensions), and metabolic modeling to put it together, and David's done a great job with it.

I'll point out that I don't believe DFBAlab, the MATLAB package, actually reformulates the problem as a DAE. I think it instead just includes lexicographic constraints (for identifiability) and an LP-feasibility solution (to prevent LP crashes), but continues to solve the LP at each ODE integration. So the more advanced algorithm, combined with a C-level integrator, would likely mean that this version greatly improves on speed. DFBAlab, while widely used, is also not technically open-source.

I've submitted a few gitlab issues on better packaging / docker distribution, but otherwise my review is finished!

marcosvital commented 4 years ago

Thank you so much for you contributions, @pstjohn.

@synchon and @jdbrunner, I see that both of you have started the revision and left a few box unchecked: do you have any specific comments about those points to the authors?

@davidtourigny, @Midnighter and @carrascomj, feel free to post here any changes you make on the manuscript and on the repository, ok?

davidtourigny commented 4 years ago

We have addressed @pstjohn 's issue on the repository https://gitlab.com/davidtourigny/dynamic-fba/-/issues/28

synchon commented 4 years ago

@marcosvital I have not completed the checks for those yet, will do it ASAP.

synchon commented 4 years ago

The package delivers what it aims to and in particular, I really like the level of abstraction used for the API design. Considering the complexity involved, it has been executed neatly.

I just have a few suggestions regarding the documentation and I have opened https://gitlab.com/davidtourigny/dynamic-fba/-/issues/29. Apart from this, I do not have any other suggestion and hence, I conclude my review.

jdbrunner commented 4 years ago

The package certainly fills a big need. For modeling interacting microbes, efficient dfba is a must, and I'm not aware of any current tools that do the job well. This package runs fast and is easy to use, with the only real drawback to simulating single models being the necessity of using docker. I am particularly impressed by how easy it is to control exchange flux lower bounds as functions of available resource.

My only comment is that the package as it currently stands does not allow for community simulations. A method for community dFBA in which microbes are coupled by a shared external metabolite pool would be a nice addition. With that suggestion, I conclude my review.

davidtourigny commented 4 years ago

Dear @synchon @pstjohn and @jdbrunner

We (@Midnighter , @carrascomj and I) thank you very much for your positive reviews and great suggestions! We hope we have sufficiently addressed the issues you have raised on the repository.

Regarding @jdbrunner 's two last points: 1) distributing python wheels cross-platform for this project is work in progress (see issues 10 and 26 that unfortunately proves particularly challenging due to the compiled dependencies), but we hope at least the docker image proves sufficient for now, or that advanced users are able to follow the instructions for building from the source distribution; 2) a multi-species extension is certainly planned long-term for this project (see issue 14), but our primary objective was to implement a robust and easy-to-use package to make the most efficient DFBA algorithms available to users, before moving on to more complex models. We hope that this serves that purpose and look forward to feedback from the community regarding development of such extensions.

marcosvital commented 4 years ago

Dear @synchon @pstjohn and @jdbrunner

Thank you all very much for reviewing this submission. If you feel that all issues you have raised are sufficiently addressed, we will soon move on to publication.

jdbrunner commented 4 years ago

Yep, I'm satisfied!

Midnighter commented 4 years ago

At @marcosvital, one of the references which is cited as a pre-print was recently accepted. I would like to edit that reference before publication when I have the full information.

davidtourigny commented 4 years ago

@Midnighter I may be wrong, but there is nothing that prevents the paper being updated post-publication is there?

pstjohn commented 4 years ago

I'm satisfied! Great work

marcosvital commented 4 years ago

@davidtourigny, @Midnighter and @carrascomj, I'll check out if we can easily edit the paper after acceptance, and will let you know.

In the meantime, please take a final look at the proof (specially if any changes were made during the reviewing process), as we are close to finish here.

marcosvital commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

synchon commented 4 years ago

Apologies for the delayed reply. I'm satisfied as well!

davidtourigny commented 4 years ago

Hi @marcosvital , the proof looks fine to me (no changes made during the review process). Thanks!

marcosvital commented 4 years ago

Hi again, @davidtourigny, @Midnighter and @carrascomj. Editing the paper after publishing is reserved for corrections, so it's probably better to wait if you are fine with it. @Midnighter, any news about the publication?

davidtourigny commented 4 years ago

I am happy to wait a couple more weeks if required (indeed, will be on vacation in any case).

Midnighter commented 4 years ago

I know that the DOI is going to be 10.15252/msb.20199235 but it's not available just yet. I think a few more days and it should be online. I'll edit the reference as soon as I know more.

Midnighter commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

PDF failed to compile for issue #2342 with the following error:

Cloning into 'tmp/2342'...

davidtourigny commented 4 years ago

That is strange. It compiles fine with the preview service...

@marcosvital apart from this it appears we are now ready!

davidtourigny commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

davidtourigny commented 4 years ago

@marcosvital all done!

marcosvital commented 4 years ago

@davidtourigny, @Midnighter and @carrascomj, we are almost done here. You will need to archive the last release of the package (on Zenodo, figshare, or other) - and if you already didn't do this, please let me know the package version number and archive DOI. Once this is done, we'll be ready to publish!

davidtourigny commented 4 years ago

@marcosvital great! We have tagged the latest release version 0.1.8 and archived it using Zendo DOI: 10.5281/zenodo.4009224.

I don't seem to be able to add this information to the origional issue comment, sorry.

marcosvital commented 4 years ago

No need to worry about it, @davidtourigny, I'll do it.

marcosvital commented 4 years ago

@whedon set v0.1.8 as version

whedon commented 4 years ago

OK. v0.1.8 is the version.

marcosvital commented 4 years ago

@whedon set 10.5281/zenodo.4009224 as archive

whedon commented 4 years ago

OK. 10.5281/zenodo.4009224 is the archive.

marcosvital commented 4 years ago

@whedon accept

whedon commented 4 years ago
Attempting dry run of processing paper acceptance...
whedon commented 4 years ago

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/1689

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/1689, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true
kthyng commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

davidtourigny commented 4 years ago

Everything looks good to me. Thanks

kthyng commented 4 years ago

A few tweaks for your paper (I'm not used to gitlab and looks harder to directly edit and submit a PR as an outside person than it is in github):

kthyng commented 4 years ago

I see the version and Zenodo archive had been added and the metadata for the archive are correct, excellent!

davidtourigny commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

davidtourigny commented 4 years ago

The latest version should take care of all your points @kthyng . Thank you