JOSS Review

[amoeba]

Hi @jfaraudo, I've started my review and will file issues here. I'm working on the review now so I'm going to edit this comment as I go.

The following review checklist items require changes:

• [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Requested Change: Please use the uppercase LICENSE for the license file's name
• [x] Paper: The paper has far too much content at this point and should be shortened considerably. Please review https://joss.readthedocs.io/en/latest/submitting.html#what-should-my-paper-contain. I think you could reduce your Introduction and Statement of Need section into just a Statement of Need section into three paragraphs, mostly containing the text at the end of the existing section. Remove Methods, Program description, use and features, Examples, Compatibility and distribution.

Non-review-checklist items:

• [ ] Update Zenodo citation to match auto-generated citation
• [x] Try to use the name "Chitin Builder" everywhere, updating any alternative usages
• [x] Had an issue opening the documentation from the Chitin Builder GUI. Filed https://github.com/soft-matter-theory-at-icmab-csic/chitin_builder/issues/4.

[amoeba]

Hi @jfaraudo, I ran into this error when I opened Chitin Builder and clicked the "Generate Chitin Structure" button. I'm having this issue both on macOS and Windows:

invalid command name "::namespace inscope :: {::chitin::replicate "$::chitin::xdim" "$::chitin::ydim" "$::chitin::zdim" "$::chitin::crystal" "$::chitin::perio"}"
invalid command name "::namespace inscope :: {::chitin::replicate "$::chitin::xdim" "$::chitin::ydim" "$::chitin::zdim" "$::chitin::crystal" "$::chitin::perio"}"
    while executing
"[namespace code {::chitin::replicate "$::chitin::xdim" "$::chitin::ydim" "$::chitin::zdim" "$::chitin::crystal" "$::chitin::perio"}]"
    invoked from within
".chitin.but46 invoke"
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 [list $w invoke]"
    (procedure "tk::ButtonUp" line 23)
    invoked from within
"tk::ButtonUp .chitin.but46"
    (command bound to event)

any ideas?

[jfaraudo]

Hello @amoeba , sorry to hear this. We had some users using WIndows that reported problems after an update 1 year ago and we have been unable to solve it , the only workout that we had was to ask users to move to Ubuntu or Mac. Up to now we had no problems reported for Mac users, so I am worried to see that you had problems in a Mac. Please, could you 1) quote the exact OS and version that you are running? 2) It looks like a permisions issue in creating the crystal structure. Maybe running VMD as superuser may help. thanks! jordi PS: Also, I am curios to know what happens if you accept (OK or skip). Are the pdb and psf files of the structure generated? and displayed? This may help to locate the issue. Thank you!!

[jfaraudo]

Hello again @amoeba , I have been able to reproduce your error on a macOS Monterey 12.4 . For some reason, in our older Mac this error does not appear. I also checked that the same error appears in WIndows 11. The code works fine in Ubuntu 18 and 20. We have found that the error appears when the code is working with a temporary file that is automatically removed before the final structure is copied to the destination file selected by the user. We believe that this can be fixed easily by a simple modification in the tcl code when dealing with the temporary files. We will try to deal with this problem as soon as possible and provide a modified tcl file that hopefully will solve this issue for Mac and windows users.

I assume that you do not have a machine with ubuntu handy, so I think the best option for you to continue your evaluation should be to wait until we post the correction .

Thanks again for your time testing the "chitin builder".

[amoeba]

Hi @jfaraudo, thanks for tracking down the issue. I can continue my review using a Linux VM for now and can help verify your fix on macOS and Windows when it's ready. I should have time to continue that this weekend. Let me know if you have any questions about my request for edits on the paper in the mean time.

[tonigi]

Here is my review and comments. (Let me know if it is not the right place).

• The plugin is indeed a relatively short piece of software. However, it is not uncommon for VMD plugins to perform useful tasks in a few lines of code. In my opinion the sheer amount of code should not be a factor in the decision for this kind of software.
• On the contrary, the ability to conveniently build structured materials for simulations is a common need for simulations. Of course every community will have their material of choice, so perhaps not many people are working on chitin, but it is very good to have a builder for it.
• I did not encounter the mentioned bugs with MacOS, except some tk-related quirkiness which is kind of expected and outside of the author's control.
• The paper reads nicely.

On the other hand, the software implementation could benefit from some additional polish and a software-savvy eye.

• As mentioned, the documentation points to an obsolete page.
• It's slow in building, even a 5x5x5 supercell. 11x11x1 takes ages on a fast machine. That's weird given that systems are < 30k atoms.
• The software structure is unnecessarily involved: there is no separation between gui and code (e.g. the "main" calculation is all done in an inline code block within the "Generate" button)
• Most important of all, there is no way to invoke the generator from the command line or other software (dual GUI and command line use, as is common for VMD plugins)
• Looking at the replicate function, which seems the main entry point, some arguments are passed by argument, some are taken from the environment.
• There are some "duplicate atom key" warnings upon second and following runs. This may suggest a skipped "resetpsf"
• Every generate requires navigating to the destination directory, every time starting from the system root. Also, clicking cancel does not abort the generation.
• The "crystal.log" file is often empty

[amoeba]

Hi @jfaraudo, I just wanted to check in on the status of (1) your updates to the paper as requested above and (2) you posting a new software release for me to test on macOS. Let me know if I can help at all such as reviewing early edits or testing early releases.

[jfaraudo]

Hi @amoeba ! We just posted a new version at the master branch at chitin github repo GitHub https://github.com/soft-matter-theory-at-icmab-csic/chitin_builder The new version works better than the previous one, it works fine in most of the tests I did in MAC but sometimes there is still an odd behaviour (not always). Sometimes there is a write error when creating the output files which seems to depend on the OS version and the name of the output folder. We noticed that when this error appears, in can be easily skipped by going to the Extensions - Tk console menu in VMD, and in the console just do a cd to the desired folder (this should be done automatically but it seems that sometimes something is wrong with the appropiate environment global variable of VMD). If the error appears and one does this trick once, the error no longer appears (even closing VMD and opening it again). We are working on fixing this (hopefully it will be done soon), but if you can try with your computer your feedback will be appreciated.

[jfaraudo]

Hi @amoeba ! We just posted our final version at the master branch at chitin github repo GitHub https://github.com/soft-matter-theory-at-icmab-csic/chitin_builder Now it works without problems in WIndows (tested 10 and 11) . In the case of MAC, depending on the OS version , the Tk console of VMD must be open during the chitin builder execution. This simply means selecting the option Tk console in the VMD Extensions menu. We added an explanation about this in the README. Now we will work on the paper changes.

[amoeba]

Thanks @jfaraudo, I'll return to my review this weekend using the new version and will let you know how that goes.

[amoeba]

Hi @jfaraudo, thanks for fixing my usage issue and for updating the paper. I reviewed the paper and the README and sent in a small PR with edits to both: https://github.com/soft-matter-theory-at-icmab-csic/chitin_builder/pull/11.

The only remaining item from my checklist is,

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

From https://joss.readthedocs.io/en/latest/review_checklist.html#documentation. Can you please add a section in the README with something simple covering this?

[jfaraudo]

@amoeba We respectfully disagree with your last comment. If you look at our README, you will see that it had a section entitled "Contributing, Reporting Issues and Support". In this section there is a short explanation and a link to an extensive document explaining in detail all these points: 1) How to contribute to the software 2) Report issues or problems with the software 3) Seek support. This is fully compliant with the journal rules and in fact it is identical to what is done exactly in other JOSS publications (see for example this recently published paper in JOSS https://github.com/cmelab/flowerMD ). So we think that the remaining item in the checklist should be closed, please.

[amoeba]

Hi @jfaraudo. My comment was made before the section was added in https://github.com/soft-matter-theory-at-icmab-csic/chitin_builder/commit/3d1cb1a250b03eeeeed28798a2a72754db07acbc but now that it has been added I've checked the item above.

My review is an Accept at this stage, see https://github.com/openjournals/joss-reviews/issues/5771#issuecomment-1894449823, and this issue can be closed if you like.

[jfaraudo]

@amoeba Ah, okay, perfect!! So I am closing this.

[jfaraudo]

No further items pending, so this issue is closed.