joss-reviews icon indicating copy to clipboard operation
joss-reviews copied to clipboard

[REVIEW]: solid_dmft: gray-boxing DFT+DMFT materials simulations with TRIQS

Open editorialbot opened this issue 2 years ago β€’ 11 comments

Submitting author: @the-hampel (Alexander Hampel) Repository: https://github.com/TRIQS/solid_dmft Branch with paper.md (empty if default branch): joss Version: 3.1.1 Editor: @rkurchin Reviewers: @amandadumi, @raghurama123, @shivupa Archive: Pending

Status

status

Status badge code:

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

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

@amandadumi & @raghurama123 & @shivupa, 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 @rkurchin 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 @raghurama123

πŸ“ Checklist for @shivupa

πŸ“ Checklist for @amandadumi

editorialbot avatar Jul 29 '22 12:07 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 avatar Jul 29 '22 12:07 editorialbot

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

OK DOIs

- 10.21468/SciPostPhys.10.5.117 is OK
- 10.1016/j.cpc.2020.107778 is OK
- 10.1103/PhysRevB.98.075129 is OK
- 10.1063/1.5144261 is OK
- 10.1088/1361-648x/ac5d1c is OK
- 10.1016/j.cpc.2016.03.014 is OK
- 10.1016/j.cpc.2015.04.023 is OK
- 10.1103/PhysRevResearch.2.033088 is OK
- 10.1103/PhysRevB.104.035102 is OK
- 10.1038/s41535-019-0145-4 is OK
- 10.1103/PhysRevB.104.165135 is OK
- 10.48550/arXiv.2206.07493 is OK
- 10.1103/PhysRevB.81.195107 is OK
- 10.1098/rspa.1963.0204 is OK
- 10.1103/RevModPhys.78.865 is OK
- 10.1103/PhysRevX.7.031013 is OK
- 10.1088/0953-8984/21/39/395502 is OK
- 10.1103/PhysRevB.47.558 is OK
- 10.1103/PhysRevB.54.11169 is OK

MISSING DOIs

- None

INVALID DOIs

- doi.org/10.1016/j.cpc.2019.07.003 is INVALID because of 'doi.org/' prefix

editorialbot avatar Jul 29 '22 12:07 editorialbot

Software report:

github.com/AlDanial/cloc v 1.88  T=2.79 s (53.4 files/s, 225833.3 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
XML                              1              0              0         609732
Python                          63           2460           4131           7380
Markdown                        14            150              0            653
INI                             15            137              0            490
CMake                           14             94            164            389
CSS                              2             80             20            354
Jupyter Notebook                 5              8           2015            339
TeX                              1             25              0            241
Bourne Shell                    13             68             60            189
YAML                             7             32              4            181
Pascal                           1             59              0            142
reStructuredText                 7             98            108            136
make                             2             31             17             70
HTML                             2              6              0             60
JavaScript                       2              0             13              2
-------------------------------------------------------------------------------
SUM:                           149           3248           6532         620358
-------------------------------------------------------------------------------


gitinspector failed to run statistical information for the repository

editorialbot avatar Jul 29 '22 12:07 editorialbot

Wordcount for paper.md is 1152

editorialbot avatar Jul 29 '22 12:07 editorialbot

@the-hampel, do fix that one flagged reference whenever you have a moment :)

rkurchin avatar Jul 29 '22 12:07 rkurchin

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

editorialbot avatar Jul 29 '22 12:07 editorialbot

@the-hampel, do fix that one flagged reference whenever you have a moment :)

Hi @rkurchin, I just fixed the one DOI that you pointed out: https://github.com/TRIQS/solid_dmft/commit/fe459150bbb4ca6e3815eec1fd8c5f33c98b14aa Thank you!

the-hampel avatar Jul 29 '22 18:07 the-hampel

Review checklist for @shivupa

Conflict of interest

  • [x] 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

General checks

  • [x] Repository: Is the source code for this software available at the https://github.com/TRIQS/solid_dmft?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [x] Contribution and authorship: Has the submitting author (@the-hampel) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • [x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] 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

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

  • [x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • [x] 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?
  • [x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • [x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • [x] 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?

shivupa avatar Aug 01 '22 17:08 shivupa

Review checklist for @amandadumi

Conflict of interest

  • [x] 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

General checks

  • [x] Repository: Is the source code for this software available at the https://github.com/TRIQS/solid_dmft?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [x] Contribution and authorship: Has the submitting author (@the-hampel) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • [x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] 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

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

  • [x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • [x] 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?
  • [x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • [x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • [x] 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?

amandadumi avatar Aug 03 '22 16:08 amandadumi

Hi reviewers (@amandadumi, @raghurama123, @shivupa), just checking in on this! Let me know if you have any questions about the review process.

rkurchin avatar Aug 15 '22 11:08 rkurchin

Yup thank you sorry just had a slammed schedule! I'll be working through this review over the coming days/weekend!

shivupa avatar Aug 18 '22 20:08 shivupa

πŸ”” Pinging reviewers(@amandadumi, @raghurama123, @shivupa), again, don't hesitate to ask if anything about the process is unclear!

rkurchin avatar Aug 23 '22 17:08 rkurchin

Hello, checking in to say I am summarizing my thoughts on the manuscript and will make progress on the code review in the following days. I had an event that needed preparation that was dictating my time, but that’s over now!

amandadumi avatar Aug 23 '22 20:08 amandadumi

Hi again all, checking in @raghurama123 on your review progress, and @the-hampel on whether you're working on the issues @shivupa filed.

rkurchin avatar Sep 07 '22 23:09 rkurchin

Hi, thank you all for your feedback so far. We (all authors) are working on the reports / issues this week and have a meeting scheduled later this week to finalize corrections.

the-hampel avatar Sep 12 '22 13:09 the-hampel

Hi, this is just to let you know that we addressed all open issues and corrected the paper (joss branch) and corresponding parts in the code on github (unstable branch).

  • Alex

the-hampel avatar Sep 16 '22 15:09 the-hampel

OK Sorry for the speed of this review, but I just wanted to say that the issues I have opened have been addressed.

I had worked through one of the tutorials (SrVO3 Metal Insulator transition), and I was able to get a little bit of experience of using the code. The example does require pretty significant computational resources, but this was mentioned in the tutorial so it wasn't a surprise to me. I think having real world examples as the tutorial rather than extremely lightweight/minimal tutorials is preferable in my eyes.

I do recommend this for acceptance (once the unstable branch is merged), and it does seem like this will make DMFT calculations more accessible.

shivupa avatar Sep 19 '22 18:09 shivupa

Wonderful, thanks @shivupa!

πŸ”” Checking in again on @raghurama123 and @amandadumi regarding their reviews...

rkurchin avatar Sep 19 '22 18:09 rkurchin

Hello,

I just need to take a look at the recent updates from @the-hampel and will finish the review by tomorrow morning.

amandadumi avatar Sep 19 '22 19:09 amandadumi

Hello,

I also want to apologize that this took me longer than intended, but thanks for the patience!

I was able to work through a different tutorial, "3. CSC with QE/W90 and HubbardI: total energy in Ce2O". This specific tutorial was reasonable in computational cost and was easy to work through. A quick look at the other tutorials suggests that these are also seemed informative and relevant to real application. I agree with @shivupa that the increased cost is offset by the insight gained. The code and documentation and use was also clearly organized and structured, with all needed information for beginning users present.

The community benefit is significant as the code is currently written, but also in the modular design and well-described "how to contribute" section. This encourages the transition from isolated in-house workflows of other groups into a more transferable and sustainable form, which will benefit the broader community who wish to use or develop DMFT approaches as well.

I do recommend this work for publication in JOSS!

amandadumi avatar Sep 20 '22 15:09 amandadumi

@the-hampel since we have two reviewers' approval, I'm going to go ahead and proceed here and just remove the third reviewer. I'll do an editorial pass over the manuscript and send any comments shortly. In the meantime, the next steps for you are:

  1. Merge any and all changes from this review into your main branch and issue a new version tag. (If you want to merge in the paper itself, you may, but it is not required that the actual manuscript live into the repo in perpetuity since JOSS will host it and you can simply add a badge link or whatever you like. But the actual changes to software and docs do need to be merged!)
  2. Create a DOI for the contents of the repo at the same commit corresponding to that version tag, e.g. using figshare or Zenodo. Please make sure that the metadata (version number, title, author list, etc.) match those of your manuscript.
  3. Post a comment here with the version number and DOI.

rkurchin avatar Sep 20 '22 18:09 rkurchin

@editorialbot remove @raghurama123 from reviewers

rkurchin avatar Sep 20 '22 18:09 rkurchin

@raghurama123 removed from the reviewers list!

editorialbot avatar Sep 20 '22 18:09 editorialbot

@editorialbot generate pdf

rkurchin avatar Sep 20 '22 18:09 rkurchin

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

editorialbot avatar Sep 20 '22 18:09 editorialbot

@editorialbot check references

rkurchin avatar Sep 20 '22 18:09 rkurchin

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

OK DOIs

- 10.21468/SciPostPhys.10.5.117 is OK
- 10.1016/j.cpc.2020.107778 is OK
- 10.1016/j.cpc.2019.07.003 is OK
- 10.1103/PhysRevB.98.075129 is OK
- 10.1063/1.5144261 is OK
- 10.1088/1361-648x/ac5d1c is OK
- 10.1016/j.cpc.2016.03.014 is OK
- 10.1016/j.cpc.2015.04.023 is OK
- 10.1103/PhysRevResearch.2.033088 is OK
- 10.1103/PhysRevB.104.035102 is OK
- 10.1038/s41535-019-0145-4 is OK
- 10.1103/PhysRevB.104.165135 is OK
- 10.48550/arXiv.2206.07493 is OK
- 10.1103/PhysRevB.81.195107 is OK
- 10.1098/rspa.1963.0204 is OK
- 10.1103/RevModPhys.78.865 is OK
- 10.1103/PhysRevX.7.031013 is OK
- 10.1088/0953-8984/21/39/395502 is OK
- 10.1103/PhysRevB.47.558 is OK
- 10.1103/PhysRevB.54.11169 is OK
- 10.1002/wcms.1340 is OK
- 10.1088/1361-648X/aa680e is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot avatar Sep 20 '22 18:09 editorialbot

Some minor editorial comments:

  • line 14: should read "...packages have begun to become..." (tense consistency)
  • 15: "self-written" is a bit of an awkward phrase. I might rephrase this to something like "...carried out by in-house codes developed and used in individual research groups."
  • 17: a bit awkward (and strictly ungrammatical) as-written here, maybe rephrase to "...whether black-box software, such as is available for DFT, is beneficial to the community" or something like "...whether black-box software, analogous to popular DFT packages, would be beneficial..."
  • 27: "end user" is a more standard phrase than "final user"
  • 27-28: suggest rephrase to "...can easily extend the functionality by modifying relevant modules in the code."
  • 30: delete "the" from "...and the applications based on TRIQS..."
  • 31: I would use the word "goal" instead of "philosophy" here, since that's more what's being expressed
  • 39 + caption of figure 1: be consistent about whether the first hyphen is present or not in "charge-self-consistent" (I lean towards including it)
  • 48: "reciprocal k-space" is redundant...either remove "reciprocal" or "k-" or if you really want both, put one in parentheses e.g. "reciprocal (k-)space"
  • 51, 63: should read "Green's functions"
  • 52: "limit statefulness to a minimum" is redundant...either replace "limit" with "keep" or remove "to a minimum"
  • 71: should read "only a few" or "very few"
  • 76-77: "open-source" should be hyphenated

rkurchin avatar Sep 20 '22 18:09 rkurchin

Dear @rkurchin, thank you for your comments on the paper! I just incorporated them in this commit.

merkelm avatar Sep 21 '22 14:09 merkelm

Great! So once you finish up the stuff in this comment we should be good to go!

rkurchin avatar Sep 21 '22 14:09 rkurchin