-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
[REVIEW]: MQT Core: The Backbone of the Munich Quantum Toolkit (MQT) #7478
Comments
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:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
|
Software report:
Commit count by author:
|
Paper file info: 📄 Wordcount for ✅ The paper includes a |
License info: ✅ License found: |
@1ucian0 & @edyounis & @josh146 - Thanks for agreeing to review this submission. As you can see above, you each should use the command As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines. The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention We aim for reviews to be completed within about 2-4 weeks. Please let me know if either of you require some more time. We can also use editorialbot (our bot) to set automatic reminders if you know you'll be away for a known period of time. Please feel free to ping me (@danielskatz) if you have any questions/concerns. |
Review checklist for @josh146Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
👋 @josh146 - thanks for getting started - is there anything blocking your progress (other than time)? |
👋 @1ucian0 & @edyounis - Can you also create your checklists (see #7478 (comment) for instructions) and get started with your reviews? Thanks. |
Review checklist for @1ucian0Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
As a reviewer, I would like to declare a potential conflicts of interest. I have code merged in repositories related to MQT: All my interaction with Technical University of Munich had been as part of my work in Qiskit (MQT is a dependant of Qiskit). I personally think the level of interaction is small and, therefore, I request a waiver for the COI. |
👋 @1ucian0 - thanks for letting us know about this. I agree that this is technically a conflict, but also agree that we can waive it, given the details as you mention above. |
While not mandatory, it is a usual practice to have a header about the license in the files affected by it. Currently, the files do not have this header. Is it an official policy about it? |
While @burgholzer is the clear main maintainer of the software, I was unable to find @robertwille in the history. Additionally, @ystade seems to have been contributing significantly in the last year. I think the paper could benefit on clarifying the author contributions and the criteria for inclusion. |
I'd be happy to add license headers to the project. Certainly does not hurt. @danielskatz what's the formal procedure for updating the code for the JOSS submission here? |
You are definitely right, and I think it would be a good idea to update the author list for this submission. Given his significant code contributions over the past year, I would like to add Yannick to the middle of the author list. Edit: I'd also be happy to add a short paragraph clarifying the inclusion of authors to the paper. @danielskatz Similar question to above: Do I simply update the |
@burgholzer - yes, you just update the paper content, and since we're in the middle of the review, make clear to the reviewers what changes you are making so they can factor that into their ongoing reviews. |
@burgholzer - and this goes for the code as well... |
Review checklist for @edyounisConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Based on the above, I added Yannick as a co-author in cda-tum/mqt-core@3481566 |
Added license headers to the files in cda-tum/mqt-core@c606fc0 and merged the latest |
👋 @burgholzer - I think you were going to work on this last week? Did anything happen? |
To be honest, not as much as I would have liked. I am on it though. |
Happy new year everyone! 🎉 I am coming back here with an update on addressing the review feedback. Finally found some time to continue working on this.
Improved upon the installation guide in the documentation and added a respective command for ensuring that everything worked in cda-tum/mqt-core#789 ✅
I added a related work section to the JOSS paper in cda-tum/mqt-core@262fd6d ✅
Tom indeed contributed significantly in the past. He is the creator of the ZX-calculus package that is part of MQT Core. I'd personally have no objections to including him in the list of authors. However, I find it challenging to define who should be an author on this submission and who should not. I'd have a tendency to keep the author list as it is, as these are the people who are still committed to further advancing MQT Core. Happy to adjust that though, if the reviewers have a different feeling. 🤷🏼♂️
I rephrased that part in the documentation as part of cda-tum/mqt-core#789. ✅
I am still working towards this one. 🔴 |
@editorialbot generate pdf |
@danielskatz I have checked the changes and updated my checklist accordingly. |
@edyounis - Thanks! Can you comment on what you think is needed to complete your checklist? If it makes sense, could you open an issue in the source repo about this? |
@editorialbot start review |
I finished my review and checked out all the items in my checklist. Thanks for the patience! @editorialbot recommend-accept |
Hey everyone 👋🏼 I have created tracking issues for the remaining open points on adding documentation for two of the subpackages of MQT Core here:
Tom (@pehamTom), the original author of the ZX-package in MQT Core, has volunteered to write the corresponding documentation. As such, I believe it is now more than justified to include Tom in the list of authors for this submission. I have updated the metadata of the paper submission in cda-tum/mqt-core@3b09c84 and rebased the paper branch on the latest main. @editorialbot generate pdf |
@danielskatz I have gone over the changes and have updated my checklist.
The new related work section looks great, and satisfies the State of the field requirement from the reviewing checklist 👍
This is much clearer now! Thanks @burgholzer, once cda-tum/mqt-core#813 cda-tum/mqt-core#814 are available this will complete my review! |
Thanks @josh146 |
@josh146 - I see one item in your checklist that still is not checked - can you check it off? |
👋 @edyounis - I see a couple of items left unchecked in your checklist - can you comment on what is needed for you to check them off? |
I finished my review and checked all the items on the checklist. @editorialbot recommend-accept |
@edyounis - thanks very much. (As two small points, editorialbot commands need to be the first thing in a new comment, and if you try this, you will fund that only editors can issue this command) |
Oh, I see. Thanks for your patience with me on this. |
👋 @josh146 - I see one item in your checklist that still is not checked - can you check it off? |
@danielskatz I've checked off my list! For this one,
it will be satisfied by the following issues being complete: |
@josh146 - normally, the checklist being complete means that the publication can go forward. So I wonder if you mean that you think these 2 issues should be closed before publication in this case, which normally would be handled by not checking off the last issue until then? |
👋 @josh146 - Can you me help me understand your thoughts on this, as requested above ☝ |
Hey @danielskatz, sorry for the delayed response. To clarify, I think I would like to see the resulting documentation as per those two issues before recommending publication, as those two issues are designed to address documentation shortfalls as pointed out in my original review, as well as to include Tom Peham as an author on the submission. @burgholzer do you have an update on when those two issues would have associated PRs (I see one is already open)? |
One is already open and being actively worked on. The other one is still on me. Given how I am currently on vacation and how next week is already quite "full", I'd say a realistic estimate is that both PRs will be done by end of the month. |
Thanks! I would be happy to approve once I get a chance to look at those two PRs when open, so I would say no need to wait to merge them |
👋 @burgholzer - are you ready to merge the 2 PRs? I think that once you do this, @josh146 can complete his review, and then we can move this forward to the acceptance and publication steps, given that the other two reviews are complete. |
Almost. I just created cda-tum/mqt-core#831, which finally adds the API documentation to the DD package. |
Submitting author: @burgholzer (Lukas Burgholzer)
Repository: https://github.com/cda-tum/mqt-core
Branch with paper.md (empty if default branch): joss-paper
Version: v2.7.0
Editor: @danielskatz
Reviewers: @1ucian0, @edyounis, @josh146
Archive: Pending
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
@1ucian0 & @edyounis & @josh146, 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 @danielskatz 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 @1ucian0
📝 Checklist for @josh146
📝 Checklist for @edyounis
The text was updated successfully, but these errors were encountered: