[REVIEW]: PYroMat: A Python package for thermodynamic properties

[editorialbot]

Submitting author: @jranalli (Joseph Ranalli)
Repository: https://github.com/chmarti1/PYroMat
Branch with paper.md (empty if default branch): joss2022
Version: 2.2.4
Editor: @jgostick
Reviewers: @espottesmith, @fwitte
Archive: 10.5281/zenodo.7262173

Status

Status badge code:

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

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

@espottesmith & @fwitte, 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 @jgostick 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 @espottesmith

[editorialbot]

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]

Software report:

github.com/AlDanial/cloc v 1.88  T=0.33 s (151.5 files/s, 63758.5 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          22           2511           6128           7678
HTML                            13            160              0           2804
TeX                              8            257              5            933
Markdown                         3             61              0            256
CSS                              1             25              5            150
XML                              1              2              0             35
YAML                             1              1              4             18
Bourne Shell                     1              3              1              4
-------------------------------------------------------------------------------
SUM:                            50           3020           6143          11878
-------------------------------------------------------------------------------


gitinspector failed to run statistical information for the repository

[editorialbot]

Wordcount for paper.md is 1471

[editorialbot]

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

OK DOIs

- 10.1109/FIE.2016.7757589 is OK
- 10.1021/ie4033999 is OK

MISSING DOIs

- None

INVALID DOIs

- https://peer.asee.org/28757 is INVALID because of 'https://doi.org/' prefix
- 10.18260/1-232083 is INVALID

[editorialbot]

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

[espottesmith]

Review checklist for @espottesmith

Conflict of interest

• I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

• I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• Repository: Is the source code for this software available at the https://github.com/chmarti1/PYroMat?
• License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• Contribution and authorship: Has the submitting author (@jranalli) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
• Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
• Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
• Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
• Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

• Installation: Does installation proceed as outlined in the documentation?
• Functionality: Have the functional claims of the software been confirmed?
• Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
• Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
• 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

Software paper

• Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
• A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
• State of the field: Do the authors describe how this software compares to other commonly-used packages?
• Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
• References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

[espottesmith]

My review is complete.

The authors have produced and described pyromat, a utility for calculation of gas-phase (and, to a certain extent, mixed gas/liquid) thermodynamic properties, based on established models. They provide data for ~1000 unique species, which include both common and uncommon gas-phase molecules and ions. The code is written clearly, and the documentation is, for the most part, quite good. While the use case for pyromat in research seems unclear to me (note: I do not study gas-phase or combustion thermochemistry), this seems an extremely necessary tool for students of chemistry/chemical engineering, as well as industrial engineers. My comments are below.

Compliments:

• It seems that, provided the user has some data available, this code is (relatively) easily extensible. It also appears to be relatively easily configurable, though better documentation on all of the configuration parameters could perhaps be desired.

Minor issues:

• The LICENSE.txt file refers to the GPL but does not reproduce the text of the license. As I understand JOSS' guidelines, this is not acceptable. I have created an issue ([Review] LICENSE file does not contain actual license text chmarti1/PYroMat#54) regarding this.
• One of the examples in the README does not work, though it does with small modifications (see [Review] Example for pyromat.info() in docs does not work chmarti1/PYroMat#55)
• In the text of the paper, references should probably be made to the models used to populate the data in pyromat.
• There are no guidelines on how an external developer should contribute to the code.

Major issues:

• There are areas where the code feels incomplete. Most critically, there is not a test suite. Validation - ensuring that you obtain expected values for a small set of materials, or ensuring that the predicted values are close to experimental values for well-studied substances like water - is essential to give the user confidence in the reported results.
• Some features also more generally feel half-finished; in apps/cycle.py, for instance, the RefrigerationCycle, OttoCycle, and DieselCycle classes are defined but not implemented.

[jranalli]

Thank you very much for your time in the review. I'd just like to clarify the two major issues:

• We have some tests stored in branch tests-jranalli. Specifically the files test_ig and test_mp1. Is there some type of documentation of that we could add that would help to satisfy this concern?
• I agree that the cycle codes are half finished. It represents a longer term goal for the project to include some more holistic calculations/educational modules. The part of the code that we're really submitting is the primary property calculation portion. Is there a way that documenting that would satisfy the concern, or would it be necessary to remove those from the library.

[jgostick]

Hi @espottesmith, great job on the review! Your efforts and haste are really appreciated.

@jranalli, regarding the comments of @espottesmith...

• I think that given what your package is about (computing thermodynamic properties) you really should have a visible and active test suite. Getting github actions to run pytest on your code for each and every commit is easy to setup now. Since you have tests already written then you just need to do some minor configuration stuff.
• Software is a work in progress and it is expect that things be added. I would suggest that you not put a wish list of future enhancements in the JOSS paper since you may never actually get them done. Just focus on what the package actually does at the moment. If these cylce calculations are not functional yet, then consider just removing them for this release, then you can add them to the package when the time comes.

BTW, the 2nd reviewer requested a few week delay in starting since they were on vacay. I expect they'll be dropping by to add their review shortly, so hopefully you can work on the current suggestions.

[chmarti1]

Thanks very much for your work. I'll make a few comments here:

With Respect to Test Suites
This idea really needs to be divided into two parts: (1) the testing required to validate a new data set that is imported into the suite, and (2) testing required to validate new class functionality or new features.

(1) New data are carefully validated against reference data provided by the same sources from which the models are entered. This is a time-consuming process - for multi-phase source data, it has to be done manually for every data source, since the raw data are being pulled from publications. However, once validated, the work is complete, and does not need to be repeated with new releases, so no automation suite is used.

We deliberately do not currently publish validation data. In older versions of the code, I used to include records of my validation work, but I have received emails from private companies that were attempting to interpret this as a warranty of some kind. My official policy is that the sources of the original data are cited, and users should always validate their calculations against their own experimental data in the context of their application.

However, the codes that were used to validate the original ideal gas model data are actually still coded directly into the classes. The ig class has a method called _ig__test(), and ig2 has _test(). The ig substances even have validation data tables embedded directly in their raw data. _ig__test() now complains about errors because it has not been updated along with major version changes, but it still successfully validates the original source data. This has even been used to demonstrate continuity errors in NIST's source data.

(2) On this front, @jranalli deserves credit for making similar arguments for some time. In brief, this is a proposal that makes sense, but to implement it well, it would require an effort at least as difficult as writing the package itself. Now that @jranalli is contributing so heavily to the project, it may make sense to include his validation suites in the main branch.

The challenge is a familiar one: the number of permutations of input/output possibilities is now so large (e.g. array/scalar, liquid/mixed/vapor/super-critical, primary/inverse properties, near limits/centered, countless permutations of units, and all of the combinations of those), that historically, hard-coded test suites have not performed well enough to justify the effort. So far, our most successful model has been to run ad-hoc targeted tests to try to break the new functionality, then @jranalli tries harder with his own test suite, then we release to the community. This workflow has functioned pretty well so far, but we are always happy to improve.

Undocumented Partially Coded Modules
True - we do have some undocumented code in the master branch. It is not currently part of package functionality. The apps module is utterly undocumented (except in-line) and is not imported by default. That module actually serves a specific (if slightly secret) purpose for our efforts to port to a web-based interface for PYroMat. The code there is part of a (still live) proof-of-concept portion of the website. It will be cleaned up in later releases, but we have decided to leave it as an easter egg for any interested users who may want to do their own developing.

LICENSE
True. That will be easily corrected.

README failure
This is actually a change in the functionality of info() in the last release. The new behavior was documented on the website, and in the handbook (see sections 2.2.3 and 2.2.4), but I forgot to update the README. Oops. Good catch!

[jranalli]

@editorialbot generate pdf

[editorialbot]

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

[jranalli]

Fixes implemented in the repository:

• License file extended
• README demo corrected
• Continuous Integration for functionality tests added
• Moved unfinished code (i.e. apps folder) to a separate branch to eliminate confusion.
• Citations added to mention of models in text of the paper (see previous post for updated PDF)

[fwitte]

Quick update from me @jgostick, @jranalli: I will start working on the review this week and hope to finish it on the weekend.

[jranalli]

Updates to the text of the paper based on comments from @fwitte. Rebuilding

[jranalli]

@editorialbot generate pdf

[editorialbot]

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

[fwitte]

Thanks for already answering my initial comments, I still have some small parts to do, which I could not finish to work on last weekend. Once I have done that, I will give my review here.

[jgostick]

Hey all, how are things progressing here?

[fwitte]

Thank you for reminding me @jgostick and thank you for your patience @jranalli. My schedule was quite packed, I want to check two more features of the package (mixtures and the water IF97) to finalize. I will let you know here after that. I hope I can fit that into my weekend.

[chmarti1]

FYI, the if97 class is deprecated. The mp1 class is now used by all multi-phase substances (including water).

[fwitte]

But it uses the IF97 equations, right? I was using the mp1 class and understood it was an implementation of the IF97 equations.

[chmarti1]

Good question. mp1 uses a generalized "Span and Wagner" equation of state that applies over both liquid and vapor phases without piece-wise transitions. If you are curious, the mp1 model is developed in some detail in chapter 7 (pg 97-114) of the PYroMat handbook.

The if97 class uses the IF97 piece-wise formulation that you probably know already. The IAPWS actually published an earlier model in 1995 using the Span and Wagner formulation, and it was updated in 2018.

The IF97 is handy for engineering use, because it includes polynomial expansions good for "inverse" property calculations, so it was the first thing I tried. If I had to write a code to model a Rankine cycle from scratch, I'd use IF97. However, Span and Wagner equations of state are now almost universal in applications like PYroMat because:

• Piece-wise approaches are heavily dependent on the substance being modeled. The regions' number, their arrangement, and even their formulation is so variable, it makes writing a generalized class a daunting task.
• Piece-wise approaches require the algorithm to determine the region of the state prior before calculating properties. If the primary properties are known, that is simple, but in any other case, even determining the region can require iteration before any properties are calculated.
• Numerous models using the Span and Wagner approach are now available in the literature, so extensibility to multiple substances is not a problem.
• For super-critical, vapor, and mixture states, (temperature, density) is a numerically superior means for determining the state. It addresses almost all of the singularities that occur near phase changes and the critical point, and it even permits the calculation of meta-stable properties. PYroMat does not currently support meta-stable states, but the underlying models are theoretically capable of estimating their properties.
• Once the potential numerical convergence problems are addressed, the numerical iteration required to specify states by (T,p) when the properties are calculated from (T,d) is not very expensive. Really, this problem has to be solved with IF97 anyway, because it uses (T,d) in the super-critical region.

[jranalli]

@editorialbot generate pdf

[editorialbot]

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

[jranalli]

@xuanxu I committed that change, but it messes up the printout in the paper, so I need to revert it:

I think the problem is just with the bot's parsing when it does the tests for valid DOIs, rather than the document rendering. The document rendered fine before, the only issue was the output from the check references command.

[jranalli]

@editorialbot generate pdf

[editorialbot]

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

[chmarti1]

Sorry about the issues with Zenodo. It looks like I had some confusion about the work flow with Zenodo. I think I've got it sorted now - my fault.

[jgostick]

Nice!

[jgostick]

@editorialbot recommend-accept

[editorialbot]

Attempting dry run of processing paper acceptance...

[editorialbot]

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

OK DOIs

- 10.1109/FIE.2016.7757589 is OK
- 10.1021/ie4033999 is OK
- 10.1021/acs.iecr.2c01427 is OK
- 10.1023/A:1022390430888 is OK
- 10.1023/A:1022310214958 is OK
- 10.1023/A:1022362231796 is OK
- 10.18434/T4D303 is OK
- 10.1038/s41586-020-2649-2 is OK

MISSING DOIs

- None

INVALID DOIs

- 10.18260/1-228757 is INVALID
- 10.18260/1-232083 is INVALID

[editorialbot]

👋 @openjournals/pe-eics, this paper is ready to be accepted and published.

Check final proof 👉📄 Download article

If the paper PDF and the deposit XML files look good in openjournals/joss-papers#3685, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

[jgostick]

Hi @openjournals/pe-eics, note that the two INVALID DOIs in the reference check are false alarms. The dois in the paper have 2 dashes, and this is breaking editorialbot when it attempts to check them.

[kyleniemeyer]

Hi @openjournals/pe-eics, note that the two INVALID DOIs in the reference check are false alarms. The dois in the paper have 2 dashes, and this is breaking editorialbot when it attempts to check them.

Thanks for the heads-up @jgostick—however, I did notice that some information was missing from those entries (notably the proceedings/book title).

@jranalli I made a few typo fixes and bibliography updates in chmarti1/PYroMat#76, could you merge that?

[jranalli]

Done

PS I just wanted to say thanks to everyone who has been part of this process! I'm really impressed by how easily, efficiently this went and how easy it was to follow. I wish more journals would use this type of review.

[kyleniemeyer]

@editorialbot generate pdf

[editorialbot]

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

[kyleniemeyer]

@editorialbot accept

[editorialbot]

Doing it live! Attempting automated processing of paper acceptance...

[editorialbot]

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦

[editorialbot]

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

0. Check final PDF and Crossref metadata that was deposited 👉 Creating pull request for 10.21105.joss.04757 joss-papers#3686
1. Wait a couple of minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.04757
2. If everything looks good, then close this review issue.
3. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? Notify your editorial technical team...

[kyleniemeyer]

@jranalli @chmarti1 congratulations on your article's publication in JOSS!

Many thanks to @espottesmith and @fwitte for reviewing this, and @jgostick for editing.

[editorialbot]

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](https://joss.theoj.org/papers/10.21105/joss.04757/status.svg)](https://doi.org/10.21105/joss.04757)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.04757">
  <img src="https://joss.theoj.org/papers/10.21105/joss.04757/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.04757/status.svg
   :target: https://doi.org/10.21105/joss.04757

This is how it will look in your documentation:

We need your help!

The Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

• Volunteering to review for us sometime in the future. You can add your name to the reviewer list here: https://joss.theoj.org/reviewer-signup.html
• Making a small donation to support our running costs here: https://numfocus.org/donate-to-joss