Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PRE REVIEW]: MacroQueue: Automating Scanning Probe Microscopy #6632

Open
editorialbot opened this issue Apr 16, 2024 · 39 comments
Open

[PRE REVIEW]: MacroQueue: Automating Scanning Probe Microscopy #6632

editorialbot opened this issue Apr 16, 2024 · 39 comments
Assignees
Labels
pre-review Python Shell TeX Track: 2 (BCM) Biomedical Engineering, Biosciences, Chemistry, and Materials

Comments

@editorialbot
Copy link
Collaborator

editorialbot commented Apr 16, 2024

Submitting author: @Brad-Goff (Brad Goff)
Repository: https://github.com/guptagroupstm/MacroQueue
Branch with paper.md (empty if default branch):
Version: v0.3.4
Editor: @Kevin-Mattheus-Moerman
Reviewers: Pending
Managing EiC: Kevin M. Moerman

Status

status

Status badge code:

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

Author instructions

Thanks for submitting your paper to JOSS @Brad-Goff. Currently, there isn't a JOSS editor assigned to your paper.

@Brad-Goff if you have any suggestions for potential reviewers then please mention them here in this thread (without tagging them with an @). You can search the list of people that have already agreed to review and may be suitable for this submission.

Editor instructions

The JOSS submission bot @editorialbot is here to help you find and assign reviewers and start the main review. To find out what @editorialbot can do for you type:

@editorialbot commands
@editorialbot editorialbot added pre-review Track: 2 (BCM) Biomedical Engineering, Biosciences, Chemistry, and Materials labels Apr 16, 2024
@editorialbot
Copy link
Collaborator Author

Hello human, 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
Copy link
Collaborator Author

Software report:

github.com/AlDanial/cloc v 1.90  T=0.09 s (1017.3 files/s, 243928.4 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          19            867            500           4079
JSON                             4              0              0           3771
SVG                              1              0              0           2671
HTML                            18            374             54           2521
XML                              1              0              0           2504
JavaScript                      12            131            221            880
CSS                              4            190             35            780
reStructuredText                16            315             72            361
TeX                              1              8              0            142
Markdown                         5             61              0            119
YAML                             2             12             12             76
Bourne Shell                     2              7              3             40
DOS Batch                        1              8              1             26
make                             1              4              7              9
-------------------------------------------------------------------------------
SUM:                            87           1977            905          17979
-------------------------------------------------------------------------------

Commit count by author:

   158	Brad
    21	guptagroupstm
     6	Sanskruti Admane
     1	zhihan

@editorialbot
Copy link
Collaborator Author

Paper file info:

📄 Wordcount for paper.md is 1020

✅ The paper includes a Statement of need section

@editorialbot
Copy link
Collaborator Author

License info:

✅ License found: MIT License (Valid open source OSI approved license)

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@Kevin-Mattheus-Moerman
Copy link
Member

@editorialbot check references

@editorialbot
Copy link
Collaborator Author

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

OK DOIs

- 10.1088/2053-1583/ad1c6d is OK
- 10.5281/zenodo.6399528 is OK
- 10.5281/zenodo.3732545 is OK
- 10.5281/zenodo.8265981 is OK
- 10.5281/zenodo.8265981 is OK
- 10.1088/2053-1583/ad1c6d is OK

MISSING DOIs

- No DOI given, and none found for title: ScopeFoundry
- No DOI given, and none found for title: WxPython
- No DOI given, and none found for title: MacroQueue Documentation
- No DOI given, and none found for title: Formation of Oriented Bilayer Motif – Vanadyl Phth...

INVALID DOIs

- None

@Kevin-Mattheus-Moerman
Copy link
Member

Kevin-Mattheus-Moerman commented Apr 16, 2024

@Brad-Goff Dear author, thanks for this submission. I am the AEiC on this track and here to help process the initial steps. Before we proceed, please can you have a look at the following points:

  • Please study the above reference check ☝️ and see if you can address any of the reported potential DOI issues. You can add/amend DOI entries in your .bib file, and call @editorialbot check references here to check them again.
  • In your affiliations, please spell out USA as United States of America

@Kevin-Mattheus-Moerman
Copy link
Member

@editorialbot assign me as editor

@editorialbot
Copy link
Collaborator Author

Assigned! @Kevin-Mattheus-Moerman is now the editor

@Kevin-Mattheus-Moerman
Copy link
Member

@uellue, @beniroquai, @untzag, @kasasxav, @ziatdinovmax, @aquilesC, @po60nani, @jingpengw, @alexriss, @DaniBodor, @caldarolamartin would you be interested in reviewing this submission on scanning probe microscopy for JOSS?

Software: https://github.com/guptagroupstm/MacroQueue
Paper: https://raw.githubusercontent.com/openjournals/joss-papers/joss.06632/joss.06632/10.21105.joss.06632.pdf

More on reviewing for JOSS:
https://joss.readthedocs.io/en/latest/reviewer_guidelines.html

If you are interested you can let me know here. Thanks!

@uellue
Copy link

uellue commented Apr 16, 2024

It sounds interesting. However, it seems that completing the review checklist requires access to a compatible SPM, which I don't have.

For that reason I won't be able to review this.

@Brad-Goff As a general comment, it seems that the software can only be controlled via a GUI? From my experience with performing non-standard transmission electron microscopy experiments, a control software should primarily offer an API, which can then be wrapped by a GUI component for convenience. That offers more flexibility and faster turn-around with new experiments and features. The way it is documented, this software is only useful for a very specific combination of devices it controls. Is it common to have exactly this setup available? How can users integrate different hardware?

@DaniBodor
Copy link

Unfortunately, I don't have time atm. Also, I don't have experience with EM, just light microscopy.

@aquilesC
Copy link

Thanks for the ping. At the moment I don't have the time for a review, but I quickly went through the sotware:

The program seems to run only on Windows, to which I don't have access (plus I don't have access to the hardware, but that I believe is a second-order issue).

I do suggest the authors run a linter/formatter on the code (such as ruff). There are excess variables (defined but never used), lines ending with a ; (which runs, but is not standard in Python), bare except statements, imports not at the top of the file, and more.

Also, I didn't see documentation on the code, and some miss-matches between british and american spelling. I think those issues could be resolved before the review starts.

@alexriss
Copy link

Unfortunately, I also don't have access to an STM at the moment.

@ziatdinovmax
Copy link

It seems that evaluating this package requires access to an actual SPM instrument, which I don't have at the moment. Is there a way to create some sort of a digital twin to test the package? I also agree with the comments above that proper control software aimed at automation should primarily offer an API, which can wrapped by a GUI for some applications.

I'm tagging @ramav87, who may be in a better position to review this paper.

@Brad-Goff
Copy link

It sounds interesting. However, it seems that completing the review checklist requires access to a compatible SPM, which I don't have.

For that reason I won't be able to review this.

@Brad-Goff As a general comment, it seems that the software can only be controlled via a GUI? From my experience with performing non-standard transmission electron microscopy experiments, a control software should primarily offer an API, which can then be wrapped by a GUI component for convenience. That offers more flexibility and faster turn-around with new experiments and features. The way it is documented, this software is only useful for a very specific combination of devices it controls. Is it common to have exactly this setup available? How can users integrate different hardware?

An SPM shouldn't be required to test the functionality of MacroQueue. In the GUI, under the Systems menu, there is a "Testing" system. In the documentation, I have a page on how to change systems. Including how to add new systems to the GUI. https://guptagroupstm.github.io/MacroQueue/Tutorials/SystemChange.html

I also should have made the distinction between MacroQueue and the functions that control the software more clear. With the first figure in the paper, I tried to explain that MacroQueue wraps functions into a GUI, and provides a queue to run the functions, but the control functions are supposed to be user-specific.

Figure3

@Brad-Goff
Copy link

Thanks for the ping. At the moment I don't have the time for a review, but I quickly went through the sotware:

The program seems to run only on Windows, to which I don't have access (plus I don't have access to the hardware, but that I believe is a second-order issue).

I do suggest the authors run a linter/formatter on the code (such as ruff). There are excess variables (defined but never used), lines ending with a ; (which runs, but is not standard in Python), bare except statements, imports not at the top of the file, and more.

Also, I didn't see documentation on the code, and some miss-matches between british and american spelling. I think those issues could be resolved before the review starts.

Thank you for the thorough suggestion. I've never used a linter before so I am going through a quick tutorial for Ruff. I'll update the code as soon as possible

@editorialbot
Copy link
Collaborator Author

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

OK DOIs

- 10.1088/2053-1583/ad1c6d is OK
- 10.5281/zenodo.6399528 is OK
- 10.5281/zenodo.3732545 is OK
- 10.5281/zenodo.8265981 is OK
- 10.5281/zenodo.8265981 is OK
- 10.1088/2053-1583/ad1c6d is OK

MISSING DOIs

- No DOI given, and none found for title: ScopeFoundry
- No DOI given, and none found for title: WxPython
- No DOI given, and none found for title: MacroQueue Documentation
- No DOI given, and none found for title: Formation of Oriented Bilayer Motif – Vanadyl Phth...

INVALID DOIs

- None

@Brad-Goff
Copy link

@editorialbot check references

@editorialbot
Copy link
Collaborator Author

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

OK DOIs

- 10.1088/2053-1583/ad1c6d is OK
- 10.5281/zenodo.6399528 is OK
- 10.5281/zenodo.3732545 is OK
- 10.5281/zenodo.8265981 is OK
- 10.5281/zenodo.8265981 is OK
- 10.1088/2053-1583/ad1c6d is OK
- 10.48550/arXiv.2403.19821 is OK

MISSING DOIs

- No DOI given, and none found for title: ScopeFoundry
- No DOI given, and none found for title: WxPython

INVALID DOIs

- None

@Brad-Goff
Copy link

@Brad-Goff Dear author, thanks for this submission. I am the AEiC on this track and here to help process the initial steps. Before we proceed, please can you have a look at the following points:

  • Please study the above reference check ☝️ and see if you can address any of the reported potential DOI issues. You can add/amend DOI entries in your .bib file, and call @editorialbot check references here to check them again.
  • In your affiliations, please spell out USA as United States of America

I've completed both.

To my knowledge, the python packages WxPython and ScopeFoundry don't have DOIs.

@Brad-Goff
Copy link

@Kevin-Mattheus-Moerman, I fixed the formatting issues. What are the next steps?

@Kevin-Mattheus-Moerman
Copy link
Member

@Brad-Goff thanks for fixing those issues. I am currently looking for reviewers. However, as you've seen, it has been difficult to find reviewers as most indicate they do not have access to the right tools/hardware to evaluate this work. I will continue to look for a bit. However, I do have to note that it has happened in the past that if no reviewers can be found that a submissions needs to unfortunately be retracted. In this case it appears to be the required hardware. Can you review the above comments (including " Is there a way to create some sort of a digital twin to test the package?") and get back with a suggestion on how to help enable review? Thanks!

@uellue
Copy link

uellue commented Apr 30, 2024

I could review parts of this since I work on automating somewhat similar experiments on scanning transmission electron microscopes. For starters, the documentation on setting up a testing system should be expanded and clarified. For me it is not at all clear what I have to put into Testing.py to make it pretend to be an instrument. This should be documented, referencing also https://joss.readthedocs.io/en/latest/review_criteria.html#api-documentation. Probably it implements some sort of abstract control interface for a real instrument? Ideally, the "Testing" instrument should be one of the options shipped with MacroQueue.

Independent of that, at least one reviewer should be familiar with SPM and test the software with at least one real instrument IMO to check off the Functionality aspect as well as State of the Field. I'll ask some people who do SPM.

@Brad-Goff
Copy link

@Kevin-Mattheus-Moerman Alright, there seems to be a miscommunication. The paper seems to be misleading. There is nothing specific to SPMs in MacroQueue. I've updated the documentation so the only place where SPMs are mentioned is in the examples section.

I've also changed the paper's title to "MacroQueue: Automating Measurements in High-Dimensional Parameter Spaces".

From the Statement of need: "The goal of MacroQueue is to provide a frontend GUI that allows users to perform measurements in high-dimensional parameter spaces without requiring the coding ability that is necessary to use the existing APIs while still providing advanced users the flexibility to write arbitrarily complex functions"

The functions currently in Testing.py and General.py are sufficient to test all of MacroQueue's functionality which include:

  • Dynamically importing python functions and adding them to the GUI
  • Grouping functions into Macros (Each macro is a type of measurement)
  • "Expanding" macros to perform measurements throughout the parameter space
  • Running the macros in the queue one at a time.
  • Create a pop-up to explain any exceptions that are raised.

If this is not enough to publish without creating a simulation of an SPM, I understand.

MacroQueueDraft.pdf

@Kevin-Mattheus-Moerman
Copy link
Member

@uellue @ziatdinovmax @alexriss thanks for getting back to me on the review invite. The authors have provided more detail above, and it looks like the software is not specific to SPM, and SPM hardware is not needed for most of the evaluation. Given the above, could you reconsider if you could review this work? Thanks again!

@Kevin-Mattheus-Moerman
Copy link
Member

@Brad-Goff thanks for the additional information. That clarifies things a lot. Can you also comment on whether Microsoft Windows is a requirement, see the comment by @aquilesC above?

@Brad-Goff
Copy link

I haven't tested it on anything other than Windows, but it shouldn't be a requirement. I'll test it and get back to you.

@uellue
Copy link

uellue commented May 3, 2024

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@uellue
Copy link

uellue commented May 3, 2024

@Kevin-Mattheus-Moerman @Brad-Goff Looking at the current documentation and the paper draft I am a bit confused what and how to even review. From what I can tell from everything discussed and shown so far, MacroQueue has three aspects or components:

  1. Macro engine and GUI to automate arbitrary data acquisition workflows in high-dimensional parameter spaces where users write their own functions to control instruments and collect data.
  2. Functions to control three particular SPMs.
  3. Functions to control other equipment.

It should be really clear what is in scope for this submission, and the code, paper and documentation should be streamlined to reflect that. If only 1. is to be reviewed, then 2. and 3. should be in a separate repository. In particular, in that case the documentation should cover creating and running your own macros extensively as primary purpose of the software, instead of mostly showing the "canned" ones for SPM.

If only 1. is the software to review, the whole thing is so far from the original submission that one could consider resubmitting after everything is cleared up and streamlined. That avoids confusion, since the original title was "Automating Scanning Probe Microscopy" which 1. has nothing to do with.

  1. and 3. as separate software package(s) would also address my concern in [PRE REVIEW]: MacroQueue: Automating Scanning Probe Microscopy #6632 (comment) about separating the GUI from the API.

If 2. and 3. are also in scope for this submission, completing the review checklist for these parts would require at least partial tests with real hardware by someone familiar with the technique, IMO.

In summary, I'd be happy to review as soon as the scope of the submission is clear enough and the paper and documentation reflect that scope well enough to form a basis for review. Right now it is so far off the mark w.r.t the review checklist that review is not possible.

@aquilesC
Copy link

aquilesC commented May 3, 2024

I haven't tested it on anything other than Windows, but it shouldn't be a requirement. I'll test it and get back to you.

@Brad-Goff : pywin32 is listed both in the setup.py and the requirements.txt file

@Brad-Goff
Copy link

@Kevin-Mattheus-Moerman @Brad-Goff Looking at the current documentation and the paper draft I am a bit confused what and how to even review. From what I can tell from everything discussed and shown so far, MacroQueue has three aspects or components:

  1. Macro engine and GUI to automate arbitrary data acquisition workflows in high-dimensional parameter spaces where users write their own functions to control instruments and collect data.
  2. Functions to control three particular SPMs.
  3. Functions to control other equipment.

It should be really clear what is in scope for this submission, and the code, paper and documentation should be streamlined to reflect that. If only 1. is to be reviewed, then 2. and 3. should be in a separate repository. In particular, in that case the documentation should cover creating and running your own macros extensively as primary purpose of the software, instead of mostly showing the "canned" ones for SPM.

If only 1. is the software to review, the whole thing is so far from the original submission that one could consider resubmitting after everything is cleared up and streamlined. That avoids confusion, since the original title was "Automating Scanning Probe Microscopy" which 1. has nothing to do with.

  1. and 3. as separate software package(s) would also address my concern in [PRE REVIEW]: MacroQueue: Automating Scanning Probe Microscopy #6632 (comment) about separating the GUI from the API.

If 2. and 3. are also in scope for this submission, completing the review checklist for these parts would require at least partial tests with real hardware by someone familiar with the technique, IMO.

In summary, I'd be happy to review as soon as the scope of the submission is clear enough and the paper and documentation reflect that scope well enough to form a basis for review. Right now it is so far off the mark w.r.t the review checklist that review is not possible.

@uellue Only #1 is part of MacroQueue. I've updated the documentation as you suggested. I've also removed the STM functions, that I was using as an example, from the repository. Do you have any other suggestions before I resubmit it?

@Kevin-Mattheus-Moerman Do you agree that it should be resubmitted to change the title?

@Brad-Goff
Copy link

I haven't tested it on anything other than Windows, but it shouldn't be a requirement. I'll test it and get back to you.

@Brad-Goff : pywin32 is listed both in the setup.py and the requirements.txt file

I just updated the repo so that it doesn't use pywin32 anymore. It should work on iOS & Linux as long as I didn't use any special Wxpython styles. I'll confirm soon.

@Brad-Goff
Copy link

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@uellue
Copy link

uellue commented May 8, 2024

Since MacroQueue is now pitched as a general-purpose tool, I've tried to figure out from the documentation how I could use it to automate parameter sweeps in TEM, which I am more familiar with. I still have no idea if this is possible and what one would have to do for that.

Since the documentation is still in such a state even after lengthy discussion and comments, I'd like to "cut my losses" and drop out from this pre-review since I don't see a perspective to complete the review checklist. I'd consider reviewing a re-submission with completely reworked documentation that is in line with the stated purpose of the software and at least offers a perspective that an average user without prior knowledge of the tool can use it for their purposes. The current state is just completely off the mark for me.

@Brad-Goff
Copy link

image

You can find the tutorial in the documentation here: https://guptagroupstm.github.io/MacroQueue.

  1. You put a python file in the Functions folder. TEM.py for your case. This is on the "Changing systems & Adding a new one" page, where there is an example of adding the "Testing" system.

  2. You put your functions that change your TEM parameters and starts the scan into this python file. This is the "Writing a new function" page, where there are 2 example functions.

image

@uellue, which step is confusing? And what's the best way to clarify it? Should I create a video that goes through it step-by-step?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pre-review Python Shell TeX Track: 2 (BCM) Biomedical Engineering, Biosciences, Chemistry, and Materials
Projects
None yet
Development

No branches or pull requests

8 participants