openjournals / joss-reviews

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

[REVIEW]: ThermoCodegen: a python/C++ package for the generation of custom thermodynamic models #4874

Closed editorialbot closed 1 year ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@mspieg<!--end-author-handle-- (Marc Spiegelman) Repository: https://gitlab.com/ENKI-portal/ThermoCodegen Branch with paper.md (empty if default branch): joss_paper Version: v1.0.0 Editor: !--editor-->@kyleniemeyer<!--end-editor-- Reviewers: @bocklund, @rdguha1995 Archive: 10.5281/zenodo.8102054

Status

status

Status badge code:

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

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

@bocklund & @rdguha1995, 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 @kyleniemeyer 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 @rdguha1995

📝 Checklist for @bocklund

editorialbot commented 1 year 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 1 year ago
Software report:

github.com/AlDanial/cloc v 1.88  T=5.18 s (316.1 files/s, 138049.3 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
HTML                            292          11712              0          96861
Python                          117          21704          26527          79399
Objective-C                      88          11791           4459          78039
Jupyter Notebook                282              0         149295          50965
C                                47           2949           1084          29397
C/C++ Header                    164           4504           2697          26051
XML                             132            463            123          24774
CSS                               8            298            168           9648
JavaScript                       10           2205           2244           8371
Cython                           12            216             95           7071
Markdown                        252           2112              0           6431
reStructuredText                 95           8034          19604           5212
LESS                             48            838            961           4524
SVG                               8              2             20           4016
C++                              14            141            452           2666
TeX                               3            244             16           1460
CMake                            11            247            332            764
YAML                             10             39             59            686
Bourne Shell                     28             78             48            641
make                              5             55             40            465
Dockerfile                        4             90            121            437
Java                              1             38             37            415
Bourne Again Shell                2              7             16             30
DOS Batch                         1              8              1             27
INI                               1              0              0              8
JSON                              1              0              0              1
--------------------------------------------------------------------------------
SUM:                           1636          67775         208399         438359
--------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago

Wordcount for paper.md is 1556

editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.5194/gmd-2-33-2009 is OK
- 10.1029/2001GC000217 is OK
- 10.1111/j.1525-1314.1998.00140.x is OK
- 10.1111/j.1525-1314.1998.00140.x is OK
- 10.1002/2016GC006702 is OK
- 10.1007/BF00307281 is OK
- 10.1111/j.1525-1314.2010.00923.x is OK
- 10.1093/petrology/29.2.445 is OK
- 10.1111/j.1365-246X.2005.02642.x is OK
- 10.1111/j.1365-246X.2010.04890.x is OK
- 10.1111/j.1365-246X.2012.05609.x is OK
- 10.1093/gji/ggx195 is OK
- 10.5281/ZENODO.5131909 is OK
- 10.6084/M9.FIGSHARE.4865333 is OK
- 10.7916/d8-wh72-vb08 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 1 year ago

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

kyleniemeyer commented 1 year ago

👋 @mspieg @bocklund @rdguha1995 the actual review will take place here.

Per the instructions above, when you are ready to do your review, comment with @editorialbot generate my checklist to generate your reviewer checklists.

rdguha1995 commented 1 year ago

Review checklist for @rdguha1995

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

kyleniemeyer commented 1 year ago

Hi @bocklund and @rdguha1995, just wanted to check on the status of your review. Would it be possible to wrap these up before the end of the month / year? Thanks!

mspieg commented 1 year ago

Thank you. I was beginning to wonder…

Marc

Sent from my iPhone

On Dec 12, 2022, at 8:09 AM, Kyle Niemeyer @.***> wrote:

 Hi @bocklund and @rdguha1995, just wanted to check on the status of your review. Would it be possible to wrap these up before the end of the month / year? Thanks!

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

rdguha1995 commented 1 year ago

Hey Team,

Apologies for the delay. I have gone through most of the paper, but I am left with some of the functional checks. I should be able to knock them off before the break.

rdguha1995 commented 1 year ago

Hey @mspieg , I am having some issues with the installation and I have opened an issue at the repository. Let me know what is going wrong here and I can continue the review from there.

mspieg commented 1 year ago

sure…thanks…

I’ll try to get to this today (but if not, both Cian and I will be in Chicago for AGU and will figure out what’s going on).

More soon Marc

On Dec 12, 2022, at 12:41 PM, rdguha1995 @.***> wrote:

Hey @mspieg https://github.com/mspieg , I am having some issues with the installation and I have opened an issue at the repository https://gitlab.com/ENKI-portal/ThermoCodegen/-/issues/12. Let me know what is going wrong here and I can continue the review from there.

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4874#issuecomment-1346940988, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKNDSGLH3S7V36BPPWBVRTWM5PVLANCNFSM6AAAAAARMOJ2QE. You are receiving this because you were mentioned.


Marc Spiegelman Dept of Earth and Env Sciences Dept of App. Physics and App. Math Columbia University

rdguha1995 commented 1 year ago

Hey Team,

I think I have gone over all the checklist items in the review. As mentioned in the issue I have opened in the target repository, the only sticking point I have is the convoluted interdependencies users might face if they decide to install the package locally. The docker containers are working great, which is what the advisable usage case is (as mentioned in the documentation), but it would be great if the authors can provide streamlined instructions for local installation as well in their documentation. A couple of potential suggestions from my end are:-

  1. Instructing the users to only try installing the package in a fresh conda environment
  2. Reducing the number of interdependent home-brew and pypi packages
  3. Having a requirements.txt file which can automatically install all dependencies.

Besides the complications in the installation, everything else looks good to me. Great job @mspieg and team for the comprehensive documentation and examples.

kyleniemeyer commented 1 year ago

Thanks @rdguha1995 for the comments!

@mspieg have you been able to take a look at the above issues?

kyleniemeyer commented 1 year ago

Hi @bocklund, do you anticipate being able to complete your review soon?

mspieg commented 1 year ago

Hi @kyleniemeyer, Cian and I are discussing appropriate additions/cautions to the installation instructions. But our primary recommendation of getting started using the containers stands (and @rdguha1995 confirms that these work out of the box) as this will still probably be the most stable solution for an arbitrary users platform. We're also waiting to hear back from @bockland. Thanks everyone for all your help.

bocklund commented 1 year ago

Review checklist for @bocklund

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

bocklund commented 1 year ago

@mspieg first, I'm deeply sorry for the delay in starting my review. Broadly, the package looks very nice and it's interesting to contrast with PyCalphad which addresses a similar problem space, but more from the materials science/metallurgy perspective and with some of the same underlying dependencies to solve key problems. Great work!

1) I'm looking at the checklist item

Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

I have found the contributing markdown document, but didn't immediately find where issues/problems should be reported or where to get support. Is that established somewhere that I'm missing.

2) Then in the paper, I think the State of the field could be better addressed. The paper does a great job enumerating some of the key features of TCG, but it could be clearer on how those features set it apart from similar tools, if any exist.

mspieg commented 1 year ago

Thanks Brandon, I’ll check on both the issues and support (but the assumption was that we would respond to issues reported on the gitlab repository, easy enough to add).

As for the state of the field, we can certainly expand on that (the only question is which field…the primary application driving TCG is still primarily computational thermodynamics of silicate systems for solid earth science for which there is relatively little comparable software, if we expand to computational thermodynamics more generally, that’s obviously a bigger space). All suggestions greatly appreciated.

Cheers Marc

On Feb 2, 2023, at 3:01 PM, Brandon Bocklund @.***> wrote:

@mspieg https://github.com/mspieg first, I'm deeply sorry for the delay in starting my review. Broadly, the package looks very nice and it's interesting to contrast with PyCalphad https://github.com/pycalphad/pycalphad which addresses a similar problem space, but more from the materials science/metallurgy perspective and with some of the same underlying dependencies to solve key problems. Great work!

I'm looking at the checklist item Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

I have found the contributing markdown document, but didn't immediately find where issues/problems should be reported or where to get support. Is that established somewhere that I'm missing.

Then in the paper, I think the State of the field could be better addressed. The paper does a great job enumerating some of the key features of TCG, but it could be clearer on how those features set it apart from similar tools, if any exist. — Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4874#issuecomment-1414296164, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKNDSHJQ2WQGYY74BJKBY3WVQHCPANCNFSM6AAAAAARMOJ2QE. You are receiving this because you were mentioned.


Marc Spiegelman Dept of Earth and Env Sciences Dept of App. Physics and App. Math Columbia University

bocklund commented 1 year ago

I think other thermodynamic software for earth science that work with related models, e.g. MELTS, probably makes sense.

mspieg commented 1 year ago

On Feb 2, 2023, at 4:12 PM, Brandon Bocklund @.***> wrote:

I think other thermodynamic software for earth science that work with related models, e.g. MELTS, probably makes sense.

Sure…and that’s easy (Mark Ghiorso is a co-author/co-developer and much of TCG and ThermoEngine really is a response to that whole family of models and optimization schemes).

I’ll take a crack at it. Let me know what you think cheers marc

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4874#issuecomment-1414379150, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKNDSB6XDS5QWURX64MRTTWVQPLVANCNFSM6AAAAAARMOJ2QE. You are receiving this because you were mentioned.


Marc Spiegelman Dept of Earth and Env Sciences Dept of App. Physics and App. Math Columbia University

kyleniemeyer commented 1 year ago

Hi @mspieg, just wanted to check back on this ☝️

mspieg commented 1 year ago

Thanks Kyle, I would definitely like to finish this off (the start of this year has been a bit crazy…I’m chair of my department) but things are settling a bit and I would like to get this tidied up shortly.

More soon marc

On Mar 1, 2023, at 10:39 AM, Kyle Niemeyer @.***> wrote:

Hi @mspieg https://github.com/mspieg, just wanted to check back on this ☝️

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4874#issuecomment-1450359036, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKNDSB22AGR5LLE3VFJJADWZ5UTVANCNFSM6AAAAAARMOJ2QE. You are receiving this because you were mentioned.


Marc Spiegelman Dept of Earth and Env Sciences Dept of App. Physics and App. Math Columbia University

kyleniemeyer commented 1 year ago

Hi @mspieg, just checking in - any chance of wrapping this up soon?

mspieg commented 1 year ago

Hi Kyle, Yes…definitely. sorry this has taken so long (my chair duties have unfortunately superseded pretty much everything on my stack at the moment but things are winding down, there is just a bit of bug-fixes needed for the next release and a small amount of additional text for the paper that needs to happen).

cc:ing Cian on this as well

More soon Marc

On May 3, 2023, at 9:44 AM, Kyle Niemeyer @.***> wrote:

Hi @mspieg https://github.com/mspieg, just checking in - any chance of wrapping this up soon?

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4874#issuecomment-1533057085, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKNDSEYI76QSGBSJEGRH4LXEJOMBANCNFSM6AAAAAARMOJ2QE. You are receiving this because you were mentioned.


Marc Spiegelman Dept of Earth and Env Sciences Dept of App. Physics and App. Math Columbia University

mspieg commented 1 year ago

Hi @kyleniemeyer, Apologies for the delay but we believe we are ready to go now. We have hopefully addressed the concerns of the reviewers and have provided updates to the paper as well as the documentation. We also provided some minor fixes to the code for the calculation of some thermodynamic parameters. We have incremented the version to v1.0.0 and updated all the containers and external code on zenodo. Please let me know if you have any other questions and hopefully we can marked this as finished.

thank you for your patience Marc

kyleniemeyer commented 1 year ago

@editorialbot generate pdf

kyleniemeyer commented 1 year ago

Great, thanks @mspieg!

@rdguha1995 and @bocklund, could you take a look and see if the changes address your comments?

editorialbot commented 1 year ago

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

rdguha1995 commented 1 year ago

The revised article looks great! In terms of installation, I think sticking to the docker container is the best strategy as @mspieg and team have mentioned through disclaimers in the repository.

All in all, this looks great! Great job @mspieg and Team!

mspieg commented 1 year ago

Thanks…much appreciated cheers Marc

On Jun 29, 2023, at 11:52 AM, rdguha1995 @.***> wrote:

The article looks great! In terms of installation, I think sticking to the docker container is the best strategy as @mspieg https://github.com/mspieg and team have mentioned through disclaimers in the repository.

All in all, this looks great! Great job @mspieg https://github.com/mspieg and Team!

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4874#issuecomment-1613448315, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKNDSHCWNKB74GFMICSXLTXNWQCZANCNFSM6AAAAAARMOJ2QE. You are receiving this because you were mentioned.


Marc Spiegelman Dept of Earth and Env Sciences Dept of App. Physics and App. Math Columbia University

bocklund commented 1 year ago

My comments about the state of the field and contributing docs are addressed as well. Cheers!

kyleniemeyer commented 1 year ago

Thanks @bocklund and @rdguha1995!

@mspieg now that the reviews are complete, we are in the endgame of accepting, with a few small steps.

kyleniemeyer commented 1 year ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

kyleniemeyer commented 1 year ago

@mspieg Please go through the above checklist 👆 and also merge the small PR I made with some minor paper fixes: https://gitlab.com/ENKI-portal/ThermoCodegen/-/merge_requests/24

mspieg commented 1 year ago

thanks Kyle, We’ll work through the checklist and hopefully have this all ready to go shortly.

Cheers Marc

On Jun 30, 2023, at 9:52 AM, Kyle Niemeyer @.***> wrote:

@mspieg https://github.com/mspieg Please go through the above checklist 👆 and also merge the small PR I made with some minor paper fixes: https://gitlab.com/ENKI-portal/ThermoCodegen/-/merge_requests/24

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4874#issuecomment-1614686253, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKNDSC75LBYBHD7RZIB3QTXN3KZ3ANCNFSM6AAAAAARMOJ2QE. You are receiving this because you were mentioned.


Marc Spiegelman Dept of Earth and Env Sciences Dept of App. Physics and App. Math Columbia University

mspieg commented 1 year ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Finished check-list. Release Version v1.0.0 tarball uploaded to zenodo at DOI 10.5281/zenodo.8102054

kyleniemeyer commented 1 year ago

@editorialbot generate pdf

kyleniemeyer commented 1 year ago

@editorialbot set 10.5281/zenodo.8102054 as archive

editorialbot commented 1 year ago

Done! archive is now 10.5281/zenodo.8102054

editorialbot commented 1 year ago

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@editorialbot commands

kyleniemeyer commented 1 year ago

@editorialbot set v1.0.0 as version

editorialbot commented 1 year ago

Done! version is now v1.0.0

editorialbot commented 1 year ago

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

kyleniemeyer commented 1 year ago

Hi @mspieg, there is a minor issue with the Connolly 2009 reference ("n/a-n/a" as the pages). Looking at the PDF at https://agupubs.onlinelibrary.wiley.com/doi/epdf/10.1029/2009GC002540 it seems they would like "Q10014" listed as the page number. Could you fix that?

mspieg commented 1 year ago

Sure…thanks for catching that.

On Jun 30, 2023, at 1:26 PM, Kyle Niemeyer @.***> wrote:

Hi @mspieg https://github.com/mspieg, there are some minor issues with the Connolly 2009 reference ("n/a-n/a" as the pages). Looking at the PDF at https://agupubs.onlinelibrary.wiley.com/doi/epdf/10.1029/2009GC002540 it seems they would like "Q10014" listed as the page number. Could you fix that?

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4874#issuecomment-1614958030, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKNDSGI2X6BP5KNUIFAXXTXN4D4FANCNFSM6AAAAAARMOJ2QE. You are receiving this because you were mentioned.


Marc Spiegelman Dept of Earth and Env Sciences Dept of App. Physics and App. Math Columbia University

mspieg commented 1 year ago

done

On Jun 30, 2023, at 1:26 PM, Kyle Niemeyer @.***> wrote:

Hi @mspieg https://github.com/mspieg, there are some minor issues with the Connolly 2009 reference ("n/a-n/a" as the pages). Looking at the PDF at https://agupubs.onlinelibrary.wiley.com/doi/epdf/10.1029/2009GC002540 it seems they would like "Q10014" listed as the page number. Could you fix that?

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4874#issuecomment-1614958030, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKNDSGI2X6BP5KNUIFAXXTXN4D4FANCNFSM6AAAAAARMOJ2QE. You are receiving this because you were mentioned.


Marc Spiegelman Dept of Earth and Env Sciences Dept of App. Physics and App. Math Columbia University

mspieg commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

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

kyleniemeyer commented 1 year ago

@editorialbot recommend-accept