Closed editorialbot closed 7 months ago
editorialbot
commented
11 months 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 commandsFor example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
@editorialbot generate pdfSoftware report:
github.com/AlDanial/cloc v 1.88 T=0.04 s (490.9 files/s, 45847.2 lines/s)
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
Markdown 12 201 0 484
TeX 2 9 0 445
C++ 1 80 88 293
YAML 2 12 4 56
Python 1 21 27 47
C/C++ Header 1 21 24 36
make 1 4 7 9
-------------------------------------------------------------------------------
SUM: 20 348 150 1370
-------------------------------------------------------------------------------
gitinspector failed to run statistical information for the repositoryReference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.1063/1.4896621 is OK
- 10.1063/5.0139699 is OK
- 10.1039/C4CP03948C is OK
- 10.1063/1.5018303 is OK
- 10.1039/C1CP22167A is OK
- 10.1016/j.bpj.2023.03.006 is OK
- 10.1063/1.1747248 is OK
- 10.1038/s41598-021-94309-y is OK
- 10.1039/C6FD00141F is OK
- 10.1016/j.softx.2015.06.001 is OK
- 10.1063/1.3578182 is OK
- 10.1016/j.cossms.2011.07.003 is OK
- 10.1063/1.3153843 is OK
- 10.1080/00268978300100971 is OK
- 10.1039/D1CP01784E is OK
- 10.1103/PhysRevLett.130.118001 is OK
- 10.1063/1.4921185 is OK
- 10.1021/acs.jpcc.5b11221 is OK
- 10.1021/ie50510a027 is OK
- 10.1016/j.actamat.2008.10.020 is OK
- 10.1016/j.actamat.2008.10.020 is OK
- 10.1016/j.jcis.2022.05.029 is OK
- 10.1063/5.0101383 is OK
- 10.1063/1.5098551 is OK
- 10.1073/pnas.1917569117 is OK
- 10.1016/j.bpj.2021.01.031 is OK
- 10.3390/biom11020278 is OK
- 10.1063/5.0101529 is OK
- 10.1063/1.1563248 is OK
- 10.1016/j.cpc.2021.108171 is OK
- 10.1063/1.445633 is OK
- 10.1063/5.0028219 is OK
MISSING DOIs
- None
INVALID DOIs
- NoneWordcount for paper.md is 2205
editorialbot
commented
11 months ago Failed to discover a valid open source license
editorialbot
commented
11 months ago :point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
lucydot
commented
11 months ago I note that although editorial-bot has not found an open source license, this project is licensed GNU GPL (with LICENSE in the repo on main branch). Editorial bot has not found it as the LICENSE is not on JOSS branch.
marjanalbooyeh
commented
11 months ago lucydot
commented
11 months ago @mattwthompson @marjanAlbouye - a reminder prompt for your reviews.
lucydot
commented
11 months ago @AndresRTejedor the Version listed in the first comment on this thread corresponds to the release version of your software (we recommend semantic versioning); can you let me know what this is?
marjanalbooyeh
commented
11 months ago @mattwthompson @marjanAlbouye - a reminder prompt for your reviews.
Thanks for the reminder @lucydot. My review is in progress, I will post it shortly.
AndresRTejedor
commented
11 months ago @AndresRTejedor the
Versionlisted in the first comment on this thread corresponds to the release version of your software (we recommend semantic versioning); can you let me know what this is?
Dear @lucydot, many thanks for your comment. We have checked the version of the software and it would correspond to the LAMMPS stable release of 2nd Aug 2023 (1.1.0) since we have just added a new functionality to this version. We will change it in the repository accordingly.
mattwthompson
commented
10 months ago main branch but is not present on the joss branch - the above issue encompasses thislucydot
commented
10 months ago We have checked the version of the software and it would correspond to the LAMMPS stable release of 2nd Aug 2023 (1.1.0) since we have just added a new functionality to this version. We will change it in the repository accordingly.
@AndresRTejedor - the version is the version of the package you are submitting (which I guess is compatible with, but distinct from, a LAMMPS release?)
marjanalbooyeh
commented
10 months ago Hi @AndresRTejedor, as part of my review, I am trying to follow the instructions here to reproduce the results of the crystal fluid interfacial free energy. First, I would like to thank you and your team for providing detailed instructions and documentations to showcase the functionalities of the Mold package.
As someone who has no experience with LAMMPS to run simulations, I was not able to proceed with these instructions (please see below). It would be great if you could add more instructions/information to the docs that could make the process easier for those (like me) who are not familiar with LAMMPS commands.
1- The bash script in utils/MI/1.Optimal_r/Run.sh requires LAMMPS.job file to submit the jobs for different radius and velocity seeds. As stated in the docs, the LAMMPS.job file depends on the user's machine, however, there is no instruction about the details of this file and what commands should be included in it. It would be great if you could provide some example/template files.
2- Step 6 in the instructions, requires compiling a fortan file named analysisNPT_clusterKoos_vec_rhodist_dellago_pressloc.10.f. I tried to compile the file with gfortran command, however, since I was not able to run the simulations in the previous step due to the missing LAMMPS.job error, the fortan file did not compile. Here is the first error:
analysisNPT_clusterKoos_vec_rhodist_dellago_pressloc.10.f:3497:9:
end do
1
Error: Expecting END SUBROUTINE statement at (1)I'm hoping once I successfully run the simulations, this error would resolve. Also, it would make it easier for the users if you add the instructions for compiling the fortan file in the docs.
Thank you!
lucydot
commented
10 months ago Hello, a quick note to say I will be on annual leave for the next two weeks - will check back here w/c 8th Jan.
AndresRTejedor
commented
10 months ago We have checked the version of the software and it would correspond to the LAMMPS stable release of 2nd Aug 2023 (1.1.0) since we have just added a new functionality to this version. We will change it in the repository accordingly.
@AndresRTejedor - the version is the version of the package you are submitting (which I guess is compatible with, but distinct from, a LAMMPS release?)
Dear @lucydot, yes the version of our software would be version 1.0.0, and it is compatible with the latest stable LAMMPS release (2nd Aug 2023).
Many thanks!
AndresRTejedor
commented
10 months ago Hi @AndresRTejedor, as part of my review, I am trying to follow the instructions here to reproduce the results of the crystal fluid interfacial free energy. First, I would like to thank you and your team for providing detailed instructions and documentations to showcase the functionalities of the Mold package.
As someone who has no experience with LAMMPS to run simulations, I was not able to proceed with these instructions (please see below). It would be great if you could add more instructions/information to the docs that could make the process easier for those (like me) who are not familiar with LAMMPS commands.
1- The bash script in
utils/MI/1.Optimal_r/Run.shrequiresLAMMPS.jobfile to submit the jobs for different radius and velocity seeds. As stated in the docs, theLAMMPS.jobfile depends on the user's machine, however, there is no instruction about the details of this file and what commands should be included in it. It would be great if you could provide some example/template files.2- Step 6 in the instructions, requires compiling a fortan file named
analysisNPT_clusterKoos_vec_rhodist_dellago_pressloc.10.f. I tried to compile the file withgfortrancommand, however, since I was not able to run the simulations in the previous step due to the missingLAMMPS.joberror, the fortan file did not compile. Here is the first error:analysisNPT_clusterKoos_vec_rhodist_dellago_pressloc.10.f:3497:9: end do 1 Error: Expecting END SUBROUTINE statement at (1)I'm hoping once I successfully run the simulations, this error would resolve. Also, it would make it easier for the users if you add the instructions for compiling the fortan file in the docs.
Thank you!
Dear @marjanAlbouye,
Thank you for your comments, I find them very useful to improve the functionality and readability of our code. In general, I will include more explicit explanations about how to run the simulations on LAMMPS, compiling the code, using the fortran and python code, etc. Regarding your specific comments:
1- The most important line you need in the submission file (LAMMPS.job) is lmp_serial -i lammps_script, where lmp_serial is the LAMMPS executable you get from LAMMPS compilation, and lammps_script is the text file that contains the simulation details (lj_moldint.in and mw_lattmold.in in our package). I will include this and the basics of this type of files in the documentation. Also, you can find more details here.
2-For compiling the fortran code you need to use the command ifort -r8 instead. I will include this command in the documentation.
Thank you for your comments!
lucydot
commented
10 months ago @editorialbot set v1.0.0 as version
editorialbot
commented
10 months ago Done! version is now v1.0.0
lucydot
commented
10 months ago @mattwthompson - a reminder prompt for your review, do you have a timescale for when this will begin?
lucydot
commented
10 months ago @AndresRTejedor, once you have updated the documentation in response to comments from @marjanAlbouye please update the thread here.
marjanalbooyeh
commented
10 months ago Hi @AndresRTejedor, as part of my review, I am trying to follow the instructions here to reproduce the results of the crystal fluid interfacial free energy. First, I would like to thank you and your team for providing detailed instructions and documentations to showcase the functionalities of the Mold package. As someone who has no experience with LAMMPS to run simulations, I was not able to proceed with these instructions (please see below). It would be great if you could add more instructions/information to the docs that could make the process easier for those (like me) who are not familiar with LAMMPS commands. 1- The bash script in
utils/MI/1.Optimal_r/Run.shrequiresLAMMPS.jobfile to submit the jobs for different radius and velocity seeds. As stated in the docs, theLAMMPS.jobfile depends on the user's machine, however, there is no instruction about the details of this file and what commands should be included in it. It would be great if you could provide some example/template files. 2- Step 6 in the instructions, requires compiling a fortan file namedanalysisNPT_clusterKoos_vec_rhodist_dellago_pressloc.10.f. I tried to compile the file withgfortrancommand, however, since I was not able to run the simulations in the previous step due to the missingLAMMPS.joberror, the fortan file did not compile. Here is the first error:analysisNPT_clusterKoos_vec_rhodist_dellago_pressloc.10.f:3497:9: end do 1 Error: Expecting END SUBROUTINE statement at (1)I'm hoping once I successfully run the simulations, this error would resolve. Also, it would make it easier for the users if you add the instructions for compiling the fortan file in the docs. Thank you!
Dear @marjanAlbouye,
Thank you for your comments, I find them very useful to improve the functionality and readability of our code. In general, I will include more explicit explanations about how to run the simulations on LAMMPS, compiling the code, using the fortran and python code, etc. Regarding your specific comments:
1- The most important line you need in the submission file (
LAMMPS.job) islmp_serial -i lammps_script, wherelmp_serialis the LAMMPS executable you get from LAMMPS compilation, andlammps_scriptis the text file that contains the simulation details (lj_moldint.inandmw_lattmold.inin our package). I will include this and the basics of this type of files in the documentation. Also, you can find more details here. 2-For compiling the fortran code you need to use the commandifort -r8instead. I will include this command in the documentation.Thank you for your comments!
Thank you @AndresRTejedor. I was able to run the optimal radius simulations following your guidelines and everything seems to be OK.
I still have some issues installing the intel FORTRAN compiler on my Linux computer to compile the analysisNPT_clusterKoos_vec_rhodist_dellago_pressloc.10.f file. I keep trying to install that, in case you know a good guideline for that as well, it would be nice to include that in the documentations as well.
AndresRTejedor
commented
10 months ago Hi @AndresRTejedor, as part of my review, I am trying to follow the instructions here to reproduce the results of the crystal fluid interfacial free energy. First, I would like to thank you and your team for providing detailed instructions and documentations to showcase the functionalities of the Mold package. As someone who has no experience with LAMMPS to run simulations, I was not able to proceed with these instructions (please see below). It would be great if you could add more instructions/information to the docs that could make the process easier for those (like me) who are not familiar with LAMMPS commands. 1- The bash script in
utils/MI/1.Optimal_r/Run.shrequiresLAMMPS.jobfile to submit the jobs for different radius and velocity seeds. As stated in the docs, theLAMMPS.jobfile depends on the user's machine, however, there is no instruction about the details of this file and what commands should be included in it. It would be great if you could provide some example/template files. 2- Step 6 in the instructions, requires compiling a fortan file namedanalysisNPT_clusterKoos_vec_rhodist_dellago_pressloc.10.f. I tried to compile the file withgfortrancommand, however, since I was not able to run the simulations in the previous step due to the missingLAMMPS.joberror, the fortan file did not compile. Here is the first error:analysisNPT_clusterKoos_vec_rhodist_dellago_pressloc.10.f:3497:9: end do 1 Error: Expecting END SUBROUTINE statement at (1)I'm hoping once I successfully run the simulations, this error would resolve. Also, it would make it easier for the users if you add the instructions for compiling the fortan file in the docs. Thank you!
Dear @marjanAlbouye, Thank you for your comments, I find them very useful to improve the functionality and readability of our code. In general, I will include more explicit explanations about how to run the simulations on LAMMPS, compiling the code, using the fortran and python code, etc. Regarding your specific comments: 1- The most important line you need in the submission file (
LAMMPS.job) islmp_serial -i lammps_script, wherelmp_serialis the LAMMPS executable you get from LAMMPS compilation, andlammps_scriptis the text file that contains the simulation details (lj_moldint.inandmw_lattmold.inin our package). I will include this and the basics of this type of files in the documentation. Also, you can find more details here. 2-For compiling the fortran code you need to use the commandifort -r8instead. I will include this command in the documentation. Thank you for your comments!Thank you @AndresRTejedor. I was able to run the optimal radius simulations following your guidelines and everything seems to be OK. I still have some issues installing the intel FORTRAN compiler on my Linux computer to compile the
analysisNPT_clusterKoos_vec_rhodist_dellago_pressloc.10.ffile. I keep trying to install that, in case you know a good guideline for that as well, it would be nice to include that in the documentations as well.
Thank you @marjanAlbouye for reporting the error. One of the authors has adapted the code so it can be compile with gfortran instead. Please try to compile again the fortran code and let me know if there is still any error. I have included a comment in the documentation in this regard.
AndresRTejedor
commented
10 months ago @AndresRTejedor, once you have updated the documentation in response to comments from @marjanAlbouye please update the thread here.
Dear @lucydot. The documentation is already updated as suggested by @marjanAlbouye.
marjanalbooyeh
commented
10 months ago Thank you @marjanAlbouye for reporting the error. One of the authors has adapted the code so it can be compile with
gfortraninstead. Please try to compile again the fortran code and let me know if there is still any error. I have included a comment in the documentation in this regard.
@AndresRTejedor Thank you. I was able to compile the fortran code after I pulled the repository, thanks for the fix. Following step 6 in optimal radius calculation documentation results in the following output, I'm not sure if it's an error or some expected behavior.
command:
bash Analysis.sh
I already changed the path variable in that file to Mold/utils/MI/1.Optimal_r/order/.
The output error is:
kT8_seed0
rm: cannot remove ‘todaslasconf.dat’: No such file or directory
rm: cannot remove ‘caja’: No such file or directory
rm: cannot remove ‘klkl’: No such file or directory
rm: cannot remove ‘kkk’: No such file or directory
At line 4539 of file analysisNPT_clusterKoos_vec_rhodist_dellago_pressloc.10.f (unit = 2, file = 'todaslasconf.dat')
Fortran runtime error: End of file
Error termination. Backtrace:
#0 0x7fbbf0b7dd36 in list_formatted_read_scalar
at ../../../libgfortran/io/list_read.c:2309
#1 0x427c4b in ???
#2 0x41d80c in ???
#3 0x42d16c in ???
#4 0x42d1df in ???
#5 0x7fbbefe69554 in ???
#6 0x400f08 in ???
#7 0xffffffffffffffff in ???
...Similar error appears for all the other seeds. However, inside each seed's directory there's nbig_seed.data file with two columns of numbers written in it. Does that mean the analysis run was successful and the results are written correctly? Thank you.
AndresRTejedor
commented
9 months ago @marjanAlbouye that's good news! From nbig_seed.data you can get the largest cluster as a function of time. If you plot the data you should get somthing similar to the figure provided in the documentation for each well radius.
marjanalbooyeh
commented
9 months ago @AndresRTejedor @lucydot Here's my review/comments. Sorry about the delay!
Paper focuses on implementing mold integrator into LAMMPS package to directly calculate the interfacial free energy between crystal and liquid/solutions. Additionaly, it includes the Lattice method for determining crystal nucleation rate.
Authors did a good job explaining the necessity of calculating the interfacial free energy, providing scientific background for both the Mold integration method and the Lattice Mold technique. The paper also discusses the limitations of the commonly used GROMACS package with tabulated potentials to run simulations, reinforcing the case for a faster and simpler implementation, namely the MOLD package.
The paper also mentions two working examples included in the package documentation that demonstrate the mold integration and the lattice mold calculation functionalities to reproduce previously published results.
Overall, I found the paper easy to understand with strong statement of the need and background sections. Including working examples that reproduce previous results shows the authors’ commitment to building a reproducible package that could benefit other researchers in the community.
Seems like the MOLD package’s primary coding contributions is around implementing the two square-like potentials available in Mold/src. Are there any additional coding contributions made by the authors to LAMMPS for implementation of the mold integration and lattice mold technique? If yes, it would be great to briefly mention them in the paper as well.
The package includes C++, Python and some Fortran code. While the functionalities of the package are tested to some extent in the two working examples, having unit tests is necessary to find bugs and assure code quality. It will also make the maintenance of the code easier. I highly suggest authors to implement some unit tests for the package.
Regarding file organization in the package, are the script files in Mold/utils specifically implemented for the two examples (i.e. Mold integration for the 100 plane of the fcc crystal and Lattice mold calculation of Ice Ih nucleation rate)? If that’s the case, it might be better to move them to the examples folder to separate the main code from example codes and makes it less confusing.
I was able to run the mold integration example with help from the author and I also noticed the documentation has improved with additional instructions. However, as mentioned earlier, it would be beneficial to include more detailed instructions, particularly targeted at readers unfamiliar with LAMMPS. I don’t have a specific suggestion for improving the documentation right now, it’s just something to keep in mind for making the package more inclusive.
Thank you.
lucydot
commented
9 months ago Thanks for your thorough review @marjanAlbouye; is there anything that is, in your view, blocking acceptance to JOSS? You mention testing. Unit tests are not absolutely necessary (but encouraged in most cases). Are there testing steps for objectively checking the expected functionality of the software (e.g., a sample input file to assert behaviour)? I assume yes given the tutorials you have worked through?
lucydot
commented
9 months ago @mattwthompson - how is your review progressing?
mattwthompson
commented
9 months ago I got it started but was quickly confused with the significant divergence in the two branches - I don't mind the paper being in a separate branch but there's quite a bit of difference in the source code and I'm not sure which I'm supposed to be looking at
AndresRTejedor
commented
9 months ago @AndresRTejedor @lucydot Here's my review/comments. Sorry about the delay!
paper summary
Paper focuses on implementing mold integrator into LAMMPS package to directly calculate the interfacial free energy between crystal and liquid/solutions. Additionaly, it includes the Lattice method for determining crystal nucleation rate.
Authors did a good job explaining the necessity of calculating the interfacial free energy, providing scientific background for both the Mold integration method and the Lattice Mold technique. The paper also discusses the limitations of the commonly used GROMACS package with tabulated potentials to run simulations, reinforcing the case for a faster and simpler implementation, namely the MOLD package.
The paper also mentions two working examples included in the package documentation that demonstrate the mold integration and the lattice mold calculation functionalities to reproduce previously published results.
Overall, I found the paper easy to understand with strong statement of the need and background sections. Including working examples that reproduce previous results shows the authors’ commitment to building a reproducible package that could benefit other researchers in the community.
Comments/Questions about the package:
* Seems like the MOLD package’s primary coding contributions is around implementing the two square-like potentials available in `Mold/src`. Are there any additional coding contributions made by the authors to LAMMPS for implementation of the mold integration and lattice mold technique? If yes, it would be great to briefly mention them in the paper as well. * The package includes C++, Python and some Fortran code. While the functionalities of the package are tested to some extent in the two working examples, having unit tests is necessary to find bugs and assure code quality. It will also make the maintenance of the code easier. I highly suggest authors to implement some unit tests for the package. * Regarding file organization in the package, are the script files in `Mold/utils` specifically implemented for the two examples (i.e. Mold integration for the 100 plane of the fcc crystal and Lattice mold calculation of Ice Ih nucleation rate)? If that’s the case, it might be better to move them to the `examples` folder to separate the main code from example codes and makes it less confusing. * I was able to run the mold integration example with help from the author and I also noticed the documentation has improved with additional instructions. However, as mentioned earlier, it would be beneficial to include more detailed instructions, particularly targeted at readers unfamiliar with LAMMPS. I don’t have a specific suggestion for improving the documentation right now, it’s just something to keep in mind for making the package more inclusive.Thank you.
Dear @marjanAlbouye Thank you for your positive feedback about the package. Please find below the reply and clarifications to your comments. • The MOLD package implements the square-well potential (in a continuous version as proposed by us (Espinosa et al., JCP 2014)) to the LAMMPS software as the fundamental tool to carry out both the mold integration and the lattice mold techniques. However, we also provide a detailed step-by-step manual, codes, analysis scripts, and input files to apply these methods and reproduce well-established values of the interfacial free energy and nucleation rates computed for two archetypal systems (water and Lennard-Jones) using our method. Hence, this package enables the implementation of a complex methodology which has been cited over the past 10 years more than 600 times in total, but that only a single group (which collaborate with us) has been able to implement with our help and by providing them with our programs. Since then, this group (Blas Lab) has published 3 articles in 3 years using this methodology (Algaba, J. et al., Romero-Guzmán, C. et al., Zerón, I. et al.) and has become a world-leading group in surface free energy calculations of methane hydrates. Hence, we believe that by making available all our codes, files, and instructions to implement the method, we open new venues for the the usage of these techniques to the scientific community. • We believe that the two different steps of the methods presented in this package implicitly serve as unit tests to check the functionalities of the code and the correct implementation/performance of the simulations. Nevertheless, following the reviewer advise, we will include more specific instructions in the documentation to specify which specific values along the two steps of the methods need to be reproduced to ensure the correct implementation and functioning of both Mold integration and the Lattice mold techniques. We thank the reviewer for this useful suggestion. • The script files included in ‘Mold/utils’ are indeed implemented specifically for the two examples. Following the reviewer advise, we have moved the codes to the examples folder and adapted the documentation accordingly. We thank the reviewer again for this useful suggestion. • Finally, we have also added some notes in the documentation to provide further details about running LAMMPS for researchers who might not be familiar with the LAMMPS Molecular Dynamics software. Moreover, we have detailed (see for instance the notes here and here) more explicitly further instructions about LAMMPS set-up and running instead of referring to its documentation so we are more inclusive to a general potential user.
AndresRTejedor
commented
9 months ago I got it started but was quickly confused with the significant divergence in the two branches - I don't mind the paper being in a separate branch but there's quite a bit of difference in the source code and I'm not sure which I'm supposed to be looking at
Dear @mattwthompson
The joss must be independent to hold the paper itself according to the submission requirements of the journal. The code and documentation of the package is in the main branch.
marjanalbooyeh
commented
9 months ago @AndresRTejedor Thank you for your response.
@lucydot The authors addressed all of comments. I was able to run the tutorials based on the instructions in the documents. I recommend acceptance.
lucydot
commented
9 months ago The joss must be independent to hold the paper itself according to the submission requirements of the journal. The code and documentation of the package is in the main branch.
We do not require a separate joss branch (your paper can be on main or equivalent). However we understand that some projects will not want a JOSS paper "cluttering" their main branch, so a separate 'joss' branch in this instance is fine.
Either way, as the @AndresRTejedor says, we expect the code and documentation for review to be in main branch. Of course, as the review proceeds, changes made be made on other development branches before merging into main.
mattwthompson
commented
9 months ago Apologies for the delay - I have a few questions but the overall work serves a useful contribution to the field. I anticipate recommending acceptance but would first like to see the answers to some of my questions.
The authors describe a software package that implements two methods for estimating an interfacial free energy. They also implement a couple of pair potentials that seem to commonly be used in these studies. The paper is a brief and effective compliment to the software and helped me, somebody with background knowledge in the topic but no specific expertise, understand the context.
The software installs fine on my computer and runs simulations without immediate issue. I didn't run anything to convergence or deeply validate the details of the pair potentials.
The software is supplemented by documentation, hosted on GitHub. There are installations instructions which, however brief, were all I needed. There is another section on how exactly to call these new pair potentials from within LAMMPS, something that a user absolutely needs. Finally, there are two long, study-scale examples of using this software to implement free energy estimates.
Comments/questions
AndresRTejedor
commented
9 months ago Apologies for the delay - I have a few questions but the overall work serves a useful contribution to the field. I anticipate recommending acceptance but would first like to see the answers to some of my questions.
The authors describe a software package that implements two methods for estimating an interfacial free energy. They also implement a couple of pair potentials that seem to commonly be used in these studies. The paper is a brief and effective compliment to the software and helped me, somebody with background knowledge in the topic but no specific expertise, understand the context.
The software installs fine on my computer and runs simulations without immediate issue. I didn't run anything to convergence or deeply validate the details of the pair potentials.
The software is supplemented by documentation, hosted on GitHub. There are installations instructions which, however brief, were all I needed. There is another section on how exactly to call these new pair potentials from within LAMMPS, something that a user absolutely needs. Finally, there are two long, study-scale examples of using this software to implement free energy estimates.
Comments/questions
- There are some references to "square-well-like" potentials (emphasis mine). I don't know what the "-like" is doing here , as far as I can tell it is simply the potential I've always understood to be called "square well."
The paper discusses a number of studies that use tabulated potentials with GROMACS. GROMACS has undergone some significant changes in the past ten years, including removing some features which have not yet been re-introduced.
- The cited papers cover a large date range, and this paper cites the GROMACS 5 paper. Specifying which GROMACS version(s) were used would be useful; just the major version would be sufficient.
- It is claimed that "... long-range corrections in energy and pressure cannot be applied across the calculation ..." As somebody who was frustrated trying to do something niche with tabulated potentials in GROMACS many years ago, I don't find this too surprising. Is this still the case? What version(s) does this apply to?
- The authors claim that simulations with tabulated potentials in GROMACS are slower than simulations in LAMMPS with an analytical potential. This is a bit surprising to me, since GROMACS is typically quite a bit faster than LAMMPS and I understand the typical slowdown associated with tabulated potentials to be less. Could you expand a bit on the performance differences here? I'm mostly curious; I don't think the choice to implement this in LAMMPS was wrong.
- The LAMMPS-facing C++ code uses the term "Pocillo" a lot. I don't know what this is, and it prevented me from gleaning from a distance what it's meant to do. It seem to translate to "mug" which might be how a square-well pair potential is described here? Whether it's jargon I'm not familiar with or a Spanish term I'm also not familiar with, a short comment ("This class implements the pair potential of x y z ...") would help people understand the code. (I take no issue with non-English languages being mixed into source code, I just want to understand what the code means, no matter the language.) I apologize if I missed a comment or nested README file somewhere.
Dear @mattwthompson ,
Thank you for your positive feedback about the package. Please find below the reply and clarifications to all your comments and questions point by point.
In some parts of the documentation, we use the term “square-like well potential” due to the difference with a proper square potential. While the latter is a discontinuous potential, and not suitable for MD simulations, our square-like well potential is continuous, and hence it can be differentiated to obtain the forces. In this context, since our potential is continuous, and thus not exactly as a square-well one, we refer with the other term. You can find a further discussion in Fig.3 of this paper which we cite in both the article and the documentation. We have added a line in the article explaining such nuance.
Thank you for raising this relevant question. The version of GROMACS we used in those works was the v4.6.7. For later versions there are some incompatibility problems when using tabular potentials (at least we found them in GROMACS 5, 2016, and 2018). We have included a sentence stating the employed version of GROMACS that we used in our former published calculations using the Mold Integration and Lattice Mold techniques.
To the best of our knowledge, for v4.6.7 no long-range corrections can be introduced when using tabular potentials. Regarding newer versions such as 2016, or 2018 (and probably also in more recent versions) the same issue still occurs. That is huge inconvenience since it disables direct comparison with standard simulations with long-range corrections using implicitly defined potentials.
As the reviewer states, LAMMPS normally performs slightly slower simulations than GROMACS (i.e., for classical water models, LAMMPS is roughly 10% slower than GROMACS; but this also depends on the version of each package). However, when using a tabular potential in GROMACS vs. an implicit potential in LAMMPS it emerges a significant difference in speed up due to the following reason. The absence of energy-pressure long-range corrections requires to significantly enlarge the potential cut-offs (e.g., from 8.5Å to 14Å in water models or electrolyte solutions; please see this, and this work for obtaining consistent values of density, potential energy, or phase diagrams among simulations using regular cut-offs (i.e., 8.5 or 9Å) and long-range corrections vs. non-long-range corrections. Hence, defining the potential intrinsically in LAMMPS solves this problem enabling long-range corrections and the use of regular cut-offs that strongly improve the LAMMPS performance vs. that of GROMACS, becoming LAMMPS ∼1.8 times faster than GROMACS.
Thank you for this comment. We used the term “pocillo” along the code with the meaning of “small well” in Spanish. We have clarified such meaning along the documentation as requested by the reviewer. Overall, we thank the reviewer for all the constructive comments to improve our manuscript/documentation.
mattwthompson
commented
9 months ago @AndresRTejedor thanks for the responses
@lucydot save the lack of low-level tests, which aren't easy to write with LAMMPS plugins and from above I gather is not essential, I was able to check off every box in the checklist. I'm happy with the authors' responses to my quibbles and therefore recommend acceptance
lucydot
commented
9 months ago That's excellent - thanks @mattwthompson and @marjanalbouye for your time and expertise ⭐
We are onto the final stages of the process now @AndresRTejedor. I'll ask editorialbot to generate the post-review checklist. You won't be able to tick off the items yourself, but let me know when each item is done.
lucydot
commented
9 months ago @editorialbot set <DOI here> as archive@editorialbot set <version here> as version@editorialbot generate pdf@editorialbot check references and ask author(s) to update as needed@editorialbot recommend-acceptlucydot
commented
9 months ago @editorialbot generate pdf
editorialbot
commented
9 months ago :point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
AndresRTejedor
commented
9 months ago @editorialbot generate pdf
editorialbot
commented
9 months ago :point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
AndresRTejedor
commented
9 months ago Post-Review Checklist for Editor and Authors
Additional Author Tasks After Review is Complete
- Double check authors and affiliations (including ORCIDs)
- Make a release of the software with the latest changes from the review and post the version number here. This is the version that will be used in the JOSS paper.
- Archive the release on Zenodo/figshare/etc and post the DOI here.
- Make sure that the title and author list (including ORCIDs) in the archive match those in the JOSS paper.
- Make sure that the license listed for the archive is the same as the software license.
Editor Tasks Prior to Acceptance
- [ ] Read the text of the paper and offer comments/corrections (as either a list or a pull request)
- [ ] Check that the archive title, author list, version tag, and the license are correct
- [ ] Set archive DOI with
@editorialbot set <DOI here> as archive- [ ] Set version with
@editorialbot set <version here> as version- [ ] Double check rendering of paper with
@editorialbot generate pdf- [ ] Specifically check the references with
@editorialbot check referencesand ask author(s) to update as needed- [ ] Recommend acceptance with
@editorialbot recommend-accept
Dear @lucydot
I have checked all the tasks. Here is the DOI of the zenodo release (10.5281/zenodo.10641293)
Many thanks for your help!
lucydot
commented
8 months ago @editorialbot generate pdf
lucydot
commented
8 months ago @editorialbot check references
editorialbot
commented
8 months ago Reference check summary (note 'MISSING' DOIs are suggestions that need verification):
OK DOIs
- 10.1063/1.4896621 is OK
- 10.1063/5.0139699 is OK
- 10.1039/C4CP03948C is OK
- 10.1063/1.5018303 is OK
- 10.1039/C1CP22167A is OK
- 10.1016/j.bpj.2023.03.006 is OK
- 10.1063/1.1747248 is OK
- 10.1038/s41598-021-94309-y is OK
- 10.1039/C6FD00141F is OK
- 10.1016/j.softx.2015.06.001 is OK
- 10.1063/1.3578182 is OK
- 10.1016/j.cossms.2011.07.003 is OK
- 10.1063/1.3153843 is OK
- 10.1080/00268978300100971 is OK
- 10.1039/D1CP01784E is OK
- 10.1103/PhysRevLett.130.118001 is OK
- 10.1063/1.4921185 is OK
- 10.1021/acs.jpcc.5b11221 is OK
- 10.1021/ie50510a027 is OK
- 10.1016/j.actamat.2008.10.020 is OK
- 10.1016/j.actamat.2008.10.020 is OK
- 10.1016/j.jcis.2022.05.029 is OK
- 10.1063/5.0101383 is OK
- 10.1063/1.5098551 is OK
- 10.1073/pnas.1917569117 is OK
- 10.1016/j.bpj.2021.01.031 is OK
- 10.3390/biom11020278 is OK
- 10.1063/5.0101529 is OK
- 10.1063/1.1563248 is OK
- 10.1016/j.cpc.2021.108171 is OK
- 10.1063/1.445633 is OK
- 10.1063/5.0028219 is OK
MISSING DOIs
- None
INVALID DOIs
- None:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
lucydot
commented
8 months ago @editorialbot set 10.5281/zenodo.10641293 as archive
Submitting author: !--author-handle-->@AndresRTejedor<!--end-author-handle-- (Andrés R. Tejedor) Repository: https://github.com/AndresRTejedor/Mold Branch with paper.md (empty if default branch): joss Version: v1.0.0 Editor: !--editor-->@lucydot<!--end-editor-- Reviewers: @mattwthompson, @marjanalbooyeh Archive: 10.5281/zenodo.10843863
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
@mattwthompson & @marjanAlbouye, 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:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @lucydot 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 @marjanAlbouye
📝 Checklist for @mattwthompson