[REVIEW comments] for the paper and code submitted to JOSS

[yw-fang]

Hello @alejandrojuria , thank you for your excellent work on this code. Here are some comments that you may consider to improve the quality of the submitted manuscript (https://github.com/openjournals/joss-reviews/issues/5810) and the features of the code.

• [x] The code, 'tightbinder', is reported to be an alternative tool that has some advantages over the popular tight binding code in the simulations of the realistic amorphous materials and the identification of the topological properties in the simulated systems. I didn't have experience with the other codes. Could you prepare a simple table or enumerated list to highlight the features of tightbinder that are not available or not easy to use in other codes? This highlight may be included in the README file. I know you have listed some functional features in the README, but let us know if these features are unique of tightbinder if you have already had the experience. I think it may help the upcoming new users to choose which codes they can take for their specific case.
• [x] In the manuscript, when mentioning the two Python classes designed for crystalline and amorphous solids, authors may include the specific names of the two classes.
• [x] On page 3 of the manuscript, change 'SK' to 'Slater-Koster' since it was only used once on line 83.
• [x] On line 95 of page 3, the authors are suggested to provide the website of the documentation which can be easily accessed by the readers.
• [x] I tested to build the documentation. I executed make html, but it raised the error: make: *** No rule to make target 'html'. Stop. make: *** No rule to make target 'html'. Stop. The "make'' refers to the version make-4.4.1 installed in my computer.
• [x] In all the tutorials, all paths for loading the files omit a dot (all my tests were run in a Linux mint system). However, if the paths are only shown for demo, it's okay and you don't need to change it.
• [x] In the tutorial for building the crystal https://tightbinder.readthedocs.io/en/latest/tutorial/crystal.html, execution of model.plot_wireframe() in Jupyter notebook raised an error TypeError: list indices must be integers or slices, not tuple'' that seems to come from ``File ~/tightbinder/tightbinder/system.py:520, in System.plot_wireframe(self, ax, linewidth, alpha, color)''.
• [x] In the tutorial for setting up the device (https://tightbinder.readthedocs.io/en/latest/tutorial/transport.html), these example codes all raised parsing error ValueError: Filling must be a number. I think there could be some errors in the examples/chain.txt.
• [x] Have you considered introducing ASE to this code (or Pymatgen or any other built-in methods in this code) so that the crystal can be built directly from the popular structure files in POSCAR and CIF? If it has been implemented, you're encouraged to mention it in the tutorial and the manuscript.
• [x] Automated tests are still lacking to verify the core functionalities (you can find below that these tests are required by Review Checklist of JOSS)

Here, I also linked this issue to the Review checklist https://github.com/openjournals/joss-reviews/issues/5810#issuecomment-1708561940 so that you can track the status of this review report.

[alejandrojuria]

Hello @yw-fang, thank you for your thorough review of the code. This past month I have been addressing the different issues and bugs you found, hopefully they are fixed now. Regarding some of your comments:

• As you say, the main purpose of the code is building Slater-Koster model, either crystalline or amorphous, and assess their topological nature. For crystalline systems, both are also done by other libraries, but as far as I know there are no libraries for building amorphous Slater-Koster models. I'm not experienced with the other codes, so the comparison is limited to what is listed on their respective websites. Instead of making a list, I have detailed more the features of the code in the README and highlighted the relevant parts, so hopefully the main characteristics come across faster.
• The instructions to build the documentation had an error, plus there was a missing dependency in the requirements.txt file. Those have been fixed now and the docs should build correctly.
• I was not familiar with ASE nor those formats you mention for describing structures. From what I read, I see that they are commonly used with other DFT codes. For this library, to build Slater-Koster models one has to specify the crystal structure together with the hopping amplitudes, and right now it cannot be done separately. However, it would make sense to be able to define the models this way, either defining the crystal algorithmically for example, or instead using those formats, and afterwards specifying the different hoppings. This is, the library does not support these formats currently but it would be reasonable to include them to facilitate interconnection with other codes and reach a wider audience.
• I have added several functional tests to the library, which can be run with pytest from the root folder of the project. These tests ensure that the functionality used for the example scripts works properly, although there are more tests than scripts to check each step required for the different types of calculations together with some core functionalities of the code.

Thank you for helping improve the code. If you have additional comments, we may discuss them here before closing the issue.

[yw-fang]

Thank you for your responses! @alejandrojuria I'll take close look at the details and get back to you soon (hopefully within this week)

[yw-fang]

@alejandrojuria

Here is an additional comment I can make based on your latest version 3a9ae67a224e4cbdf9011b65fdfac239e998501a:

make html now works after you fixed it. But I noticed that it requires several depdendencies installed. If you dislike including them in the requirements.txt, you'd better indicate them in the README.md file. In my case, I found I had to install these libraries via pip: pip install sphinx_rtd_theme pip install sphinx-autodoc-typehints pip install pydata_sphinx_theme

[alejandrojuria]

As you say, it is necessary to install additional dependencies to be able to build the documentation. I added the documentation-specific requirements in a file within docs/ but it is true I forgot to mention that in the README. I have changed the README accordingly. In fact, I have removed the package requirements.txt file given that the dependencies are now included directly in the pyproject.toml file, as the other reviewer suggested.

[yw-fang]

Perfect! I'll close this issue!