Closed whedon closed 2 years ago
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @awvwgk, @wcwitt 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:
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
Wordcount for paper.md
is 1061
Software report (experimental):
github.com/AlDanial/cloc v 1.88 T=0.27 s (230.0 files/s, 51273.1 lines/s)
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
Fortran 90 30 1725 1748 6339
TeX 5 147 0 1406
Markdown 19 219 0 1386
Bourne Shell 1 54 92 169
YAML 3 26 5 106
make 3 64 18 97
-------------------------------------------------------------------------------
SUM: 61 2235 1863 9503
-------------------------------------------------------------------------------
Statistical information for the repository 'd500ae595a303c04f7b87f2a' was
gathered on 2022/01/07.
No commited files with the specified extensions were found.
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- None
MISSING DOIs
- 10.3389/fmats.2019.00123 may be a valid DOI for title: Coupled cluster theory in materials science
- 10.1016/j.cpc.2009.06.022 may be a valid DOI for title: Ab initio molecular simulations with numeric atom-centered orbitals
- 10.1103/physrev.46.618 may be a valid DOI for title: Note on an approximation treatment for many-electron systems
- 10.1063/1.3043729 may be a valid DOI for title: The ground state correlation energy of the random phase approximation from a ring coupled cluster doubles approach
- 10.1016/s0009-2614(89)87395-6 may be a valid DOI for title: A fifth-order perturbation comparison of electron correlation theories
- 10.1063/1.455269 may be a valid DOI for title: An efficient reformulation of the closed-shell coupled cluster single and double excitation (CCSD) equations
- 10.1063/1.4977994 may be a valid DOI for title: Low rank factorization of the Coulomb integrals for periodic coupled cluster theory
- 10.1088/1367-2630/14/5/053020 may be a valid DOI for title: Resolution-of-identity approach to Hartree–Fock, hybrid density functionals, RPA, MP2 and GW with numeric atom-centered orbital basis functions
- 10.1021/acs.jpclett.0c00481.s001 may be a valid DOI for title: Accuracy of Localized Resolution of the Identity in Periodic Hybrid Functional Calculations with Numerical Atomic Orbitals
- 10.1021/acs.jpclett.8b03679.s001 may be a valid DOI for title: Physisorption of water on graphene: subchemical accuracy from many-body electronic structure methods
- 10.1063/1.5055706 may be a valid DOI for title: Reaction energetics of hydrogen on Si (100) surface: A periodic many-electron theory study
- 10.1103/physrevb.101.165138 may be a valid DOI for title: Electronic structure of bulk manganese oxide and nickel oxide from coupled cluster theory
- 10.1021/acs.jctc.7b00049 may be a valid DOI for title: Gaussian-based coupled-cluster theory for the ground-state and band structure of solids
- 10.1021/acs.jctc.5b00422 may be a valid DOI for title: Can single-reference coupled cluster theory describe static correlation?
- 10.1103/physrevlett.117.133002 may be a valid DOI for title: Towards efficient orbital-dependent density functionals for weak and strong correlation
- 10.1021/acs.jpca.9b03976.s001 may be a valid DOI for title: The CUAGAU Set of Coupled-Cluster Reference Data for Small Copper, Silver, and Gold Compounds and Assessment of DFT Methods
- 10.1063/1.1388045 may be a valid DOI for title: Extended benchmark studies of coupled cluster theory through triple excitations
- 10.1063/1.4985878 may be a valid DOI for title: Properties of the water to boron nitride interaction: From zero to two dimensions with benchmark accuracy
- 10.1103/physrevb.101.241113 may be a valid DOI for title: First-principles coupled cluster theory of the electronic spectrum of transition metal dichalcogenides
- 10.1103/physrevlett.52.1830 may be a valid DOI for title: Transition-metal monoxides: band or Mott insulators
- 10.1103/physrevb.54.1703 may be a valid DOI for title: Separable dual-space Gaussian pseudopotentials
- 10.1103/physrevb.73.035404 may be a valid DOI for title: Comparison of the full-potential and frozen-core approximation approaches to density-functional calculations of surfaces
- 10.1016/0022-3093(95)00355-x may be a valid DOI for title: Ab initio molecular dynamics for liquid metals
- 10.1039/9781849734790-00168 may be a valid DOI for title: On choosing the best density functional approximation
- 10.1007/128_2014_600 may be a valid DOI for title: Judging density-functional approximations: Some pitfalls of statistics
- 10.1021/acs.jctc.9b00957.s001 may be a valid DOI for title: Integral-direct and parallel implementation of the CCSD (T) method: Algorithmic developments and large-scale applications
- 10.1021/acs.jctc.9b00511.s001 may be a valid DOI for title: Approaching the basis set limit of CCSD (T) energies for large molecules with local natural orbital coupled-cluster methods
- 10.1021/acs.jctc.0c01129.s001 may be a valid DOI for title: Scalable Electron Correlation Methods. 8. Explicitly Correlated Open-Shell Coupled-Cluster with Pair Natural Orbitals PNO-RCCSD (T)-F12 and PNO-UCCSD (T)-F12
- 10.1063/1.464746 may be a valid DOI for title: The equation of motion coupled-cluster method. A systematic biorthogonal approach to molecular excitation energies, transition probabilities, and excited state properties
- 10.1103/physrevb.59.1758 may be a valid DOI for title: From ultrasoft pseudopotentials to the projector augmented-wave method
- 10.1088/1367-2630/17/9/093020 may be a valid DOI for title: Accurate localized resolution of identity approach for linear-scaling hybrid density functionals and for many-body perturbation theory
- 10.1002/jcc.23284 may be a valid DOI for title: Attractive electron–electron interactions within robust local fitting approximations
- 10.1063/1.1679012 may be a valid DOI for title: Coulombic potential energy integrals and approximations
- 10.1016/0009-2614(93)87156-w may be a valid DOI for title: Use of approximate integrals in ab initio theory. An application in MP2 energy calculations
- 10.1021/acs.jctc.0c00101 may be a valid DOI for title: Excitons in solids from periodic equation-of-motion coupled-cluster theory
INVALID DOIs
- None
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
@Evmoerman please do add in those missing DOI's when you get a chance!
@rkurchin Sure, I will take care of the DOIs today or tomorrow. @awvwgk I have direct contact to the maintainers/owners of both codes. Hence, if you tell me exactly to which institution/individual access to CC4S/FHI-aims should be given, this can be easily arranged.
@Evmoerman Thanks for the offer, I was just about to write Volker Blum whether I can get access to FHI-aims in some way. I'm currently in Bonn at the Mulliken Center for Theoretical Chemistry, in Stefan Grimme's group.
@rkurchin Missing DOIs have been added. @awvwgk Emails have been sent to the respective people in charge. Hopefully, by next week you will have access to both codes.
@whedon generate pdf
@whedon commands
Here are some things you can ask me to do:
# List Whedon's capabilities
@whedon commands
# List of editor GitHub usernames
@whedon list editors
# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers
EDITORIAL TASKS
# Compile the paper
@whedon generate pdf
# Compile the paper from alternative branch
@whedon generate pdf from branch custom-branch-name
# Ask Whedon to check the references for missing DOIs
@whedon check references
# Ask Whedon to check repository statistics for the submitted software
@whedon check repository
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
@whedon check references
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.3389/fmats.2019.00123 is OK
- 10.1016/j.cpc.2009.06.022 is OK
- 10.1103/physrev.46.618 is OK
- 10.1063/1.3043729 is OK
- 10.1016/s0009-2614(89)87395-6 is OK
- 10.1063/1.455269 is OK
- 10.1063/1.4977994 is OK
- 10.1088/1367-2630/14/5/053020 is OK
- 10.1021/acs.jpclett.0c00481 is OK
- 10.1021/acs.jpclett.8b03679 is OK
- 10.1063/1.5055706 is OK
- 10.1103/physrevb.101.165138 is OK
- 10.1021/acs.jctc.7b00049 is OK
- 10.1021/acs.jctc.5b00422 is OK
- 10.1103/physrevlett.117.133002 is OK
- 10.1021/acs.jpca.9b03976 is OK
- 10.1063/1.1388045 is OK
- 10.1063/1.4985878 is OK
- 10.1103/physrevb.101.241113 is OK
- 10.1103/physrevlett.52.1830 is OK
- 10.1103/physrevb.54.1703 is OK
- 10.1103/physrevb.73.035404 is OK
- 10.1016/0022-3093(95)00355-x is OK
- 10.1039/9781849734790-00168 is OK
- 10.1007/128_2014_600 is OK
- 10.1021/acs.jctc.9b00957 is OK
- 10.1021/acs.jctc.9b00511 is OK
- 10.1021/acs.jctc.0c01129 is OK
- 10.1063/1.464746 is OK
- 10.1103/physrevb.59.1758 is OK
- 10.1088/1367-2630/17/9/093020 is OK
- 10.1002/jcc.23284 is OK
- 10.1063/1.1679012 is OK
- 10.1016/0009-2614(93)87156-w is OK
- 10.1021/acs.jctc.0c00101 is OK
MISSING DOIs
- None
INVALID DOIs
- None
@awvwgk I have fixed the compilation issues with GCC-11. Tell me please, if you still experience issues with this GCC version.
@awvwgk You should have received invitations to both FHI-aims and CC4S. Have they arrived? Tell me, if you experience issues with respect to these codes.
Thanks, I'll try to build and test everything ASAP, latest next weekend ;).
@Evmoerman I don't have FHI-aims or CC4S either, sorry. Would you mind getting me access? I'm at Cambridge in the Materials Theory Group.
@wcwitt Of course. I will send a couple of emails in a minute.
Updated my review in the description above. In summary, the documentation is a bit brief
@awvwgk Thank you very much for your comments. I have already been thinking about your 1st and the 3rd point. However, - as you also state in the issue - I'm not sure if it is possible/makes sense to offer installation on windows because, as far as I know, FHI-aims and CC4S both assume a Linux environment. However, I will have a look and try to add documentation/tutorials where possible.
Thanks again.
In any case, clearly stating that the project is only officially supported on Linux (or Unix) systems will help to set the right expectations for users.
Got it. I will certainly do that.
👋 @wcwitt, just checking in whether you've been able to start your review yet!
I haven't, apologies, but I will ASAP.
On Thu, Jan 20, 2022 at 10:23 PM Rachel Kurchin @.***> wrote:
👋 @wcwitt https://github.com/wcwitt, just checking in whether you've been able to start your review yet!
— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4040#issuecomment-1017979234, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACXHHTSXWI6Q45ZPAEZUXTDUXCDMPANCNFSM5LPVWRIA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID: @.***>
:wave: @awvwgk, please update us on how your review is going (this is an automated reminder).
:wave: @wcwitt, please update us on how your review is going (this is an automated reminder).
I've managed to install the interface package, and I'm working on installing the rest (it took a while to get FHI-aims - thanks @Evmoerman for your help with that!).
My main initial reaction is that a full example is really necessary (also mentioned by @awvwgk https://gitlab.com/moerman1/fhi-cc4s/-/issues/5).
The paper and documentation also suggest it should be straightforward to use this interface with codes other than FHI-aims, but I would prefer to see evidence if possible. I can see that real effort went into this code, which I'd like to see rewarded, but if it's mainly an extension of FHI-aims I'm not certain it meets the eligibility criteria. Here is an issue to discuss (https://gitlab.com/moerman1/fhi-cc4s/-/issues/6).
@wcwitt Currently, CC-aims is only used in combination with FHI-aims. This is merely the result of me working at the Fritz-Haber-Institute, in the group which develops the FHI-aims code. However, there is nothing in CC-aims, which assumes that FHI-aims is the ab-initio code being used. The quantities required by CC-aims can be provided by any code which meets the criteria stated in the documentation. The idea is, that once CC-aims is published other localized-orbital codes will either interface to it and/or use it as a template for similar interfaces.
However, there is nothing in CC-aims, which assumes that FHI-aims is the ab-initio code being used. The quantities required by CC-aims can be provided by any code which meets the criteria stated in the documentation. The idea is, that once CC-aims is published other localized-orbital codes will either interface to it and/or use it as a template for similar interfaces.
On that note it might be worth to compare to Crystal/Cryscor (Phys. Chem. Chem. Phys., 2012,14, 7615-7628) since the general design of FHI-aims/CC-aims seems similar on the first glance. Also, what (open source) QC programs would be primary targets for interfacing with CC-aims?
Indeed, CRYSTAL would be a great candidate, as it provides a localized basis+density fitting but - as far as I remember - only offers MP2. Hence, an interface to CC4S would expand its range of functionality immensely. Another code, which would very likely benefit from an interface like CC-aims is QuantumATK, which is not open-source but free-ware for academic use. It is hard to come up with a list of truly open-source quantum-chemistry programs, as they are comparably rare. One such code, is PySCF. While PySCF offers a range of beyond-DFT methods, I personally experienced them to be unbelievably memory-demanding for bigger calculations (even for HPC standards), while the same calculations could be reasonably performed with CC4S. In this case, other codes could benefit from CC-aims/CC4S even though the codes themselves include beyond-DFT methods as I point out in the final paragraph of my manuscript.
Hi all, just wanted to check in on the status here. Seems there are at least those two issues open in the repo: one regarding a full example with FHI-aims, and one regarding interfacing to another code as this is a "selling point" of the package. From my perspective, the first one should definitely be fulfilled before acceptance. The second one would definitely be nice as well, but I'd like perspective both from reviewers on how necessary they see it to be and from @Evmoerman on how much work this would represent...
@rkurchin Yes, I fully agree. While I cannot provide an example except for FHI-aims, I of course will include a tutorial/example page on the website and I also believe that contributing guidelines, like @awvwgk suggested, are also necessary. I'm sorry I haven't taken care of these issues yet, because I'm currently visiting the CC4S Group in Vienna, where I help developing. I am certainly aware of the to do list, but the last 2 weeks were a bit stressful because I want to make the most of my short visit here. I will take care of the review issues as soon as I can. Sorry again for the delay.
Thanks - my main update is I've managed to install FHIaims with CC-aims and am looking forward to the example. @Evmoerman, I doubt any of us are worried about delays so all good. @rkurchin, I will think about your question.
@Evmoerman, no problem, take your time and make the most out of your visit.
Regarding functionality testing, I already tried setting up a calculation myself, but since I'm neither familiar with FHI-aims nor CC4S it takes some time to find the right information in the manual. Some points I stumbled upon:
I didn't really try to create an input myself yet and run a somewhat bigger test calculation, mainly because this would involve a lot of trial and error based on few information I could gather from the test-suite/source code.
@awvwgk Yea, I can see how this may be difficult at first. While invoking a CC-aims calculation/post-processing is controlled by only 1 or 2 keywords in FHI-aims, it isn't as trivial setting up the rest of the input (including the geometry.in you mentioned). While the test-inputs contain a variety of suitable inputs for FHI-aims+CC-aims, and I will certainly use one/some of these for the tutorial section, you are totally right that a separate chapter in the CC-aims documentation should exist for that. Finally, the CC-aims-related keywords are not yet documented in the FHI-aims manual because I was suspecting that the details about CC-aims may change in the course of the JOSS review, so that I didn't want to expose new potential users of FHI-aims to a functionality, which is not yet in its final stage and tried to document it as precisely as possible in the CC-aims documentation.
Hi again all, just wanted to check in here. @Evmoerman, I don't know how long your visit in Vienna is, so if you still need some more time it's no problem at all.
Hey @rkurchin. My visit here is until end of this month, but I actually started already (slowly) to fix issues with CC-aims. This week (fingers crossed) the CC4S repository goes public and I am currently adapting the file formats and the CI/CD pipeline to the new CC4S infrastructure. After the testing suite is stable and compatible again with the released CC4S, I hopefully can take care of the documentation/tutorial issues.
Hey guys, @rkurchin , @awvwgk , @wcwitt , I have now adapted the interface to work with the now public version of CC4S, which can be found here. The tests and pipelines work and use the newest version of CC4S. If nothing breaks in the meantime, I hope that I can start to take care of the issues opened so far.
Sounds great. Once there is an example, I'll work through it and finish with any other comments.
Just tried to compile CC4S again and failed with a more recent version of GCC. The overall experience of building CC4S is a bit unsettling, having a makefile invoke CMake and autotools to build dependencies seems rather fragile to me, also I have a version of yaml-cpp around which I prefer to use rather than compile yet another one. This is not really on CC-aims but more on CC4S.
I really appreciate a high-level build system (meson, CMake, or even autotools) over a bunch of makefiles. Chances that a makefile just fails are rather high (at least my success rate with makefiles has been rather low so far).
@awvwgk Thanks for your assessment. I, of course, cannot guarantee that the current installation infrastructure will change in the near future but I will forward your comment to the CC4S development team. If possible, it would probably also be a good idea to open an issue on the public cc4s repo. Is it okay if I include your e-mail address, so that someone can contact you for that issue?
I can't open issues at the CC4S GitLab, so feel free to open an issue on my behalf.
Another thing I would appreciate would be a released version of CC4S (maybe also CTF). A C++/MPI program should be easy enough to package on conda-forge, which in turn would simplify the CC-aims installation because dependencies could be installed from a package manager (for HPC usage it is of course preferable to compile from source and tune for the architecture). Libraries like ELSI and similar are already available via conda-forge.
I see. I assume that Issues have not been enabled yet for everyone so early after the code release. I have forwarded your bug report+installation recommendations to the devs. I will also inform them about your advice to package CC4S.
@awvwgk, I just got word back:
Thanks a lot.
Utilization of a custom yaml-cpp: I'm afraid it is not entirely clear, what the issue there is. Is CC4S not compiling with your version of the yaml-library? Or are you missing a straight-forward possibility to make CC4S even use your own yaml-library? You would need to elaborate on that.
I was surprised to get a full build of yaml-cpp
when starting the makefile, while I have an installation of yaml-cpp
already on my system. I could probably hack this myself into the makefile (not a big fan of this solution), but I would expect a build system to prefer installed versions over vendored ones by default.
I'm afraid for now there is only the "hacky" way to force CC4S to use custom dependencies. Furthermore, if I was informed correctly, you can now register on the public CC4S repo, which allows you to open issues.
Hi all, just wanted to check in on this – reviewers, have you been able to get the software running to check the functionality claims?
Submitting author: !--author-handle-->@Evmoerman<!--end-author-handle-- (Evgeny Moerman) Repository: https://gitlab.com/moerman1/fhi-cc4s Branch with paper.md (empty if default branch): Version: 1.0.1 Editor: !--editor-->@rkurchin<!--end-editor-- Reviewers: @awvwgk, @wcwitt Archive: 10.5281/zenodo.6658117
: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 badge code:
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
@awvwgk & @wcwitt, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
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 ✨
Review checklist for @awvwgk
✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
Review checklist for @wcwitt
✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper