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]: BellDiagonalQudits: A package for entanglement analyses of mixed maximally entangled qudits #4924
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:
|
|
Wordcount for |
|
^-- Those Biblio errors definitely need fixing @kungfugo |
@jarvist Thanks for pointing out. .bib now contains the correct DOIs. |
Review checklist for @Roger-luoConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Review checklist for @meandmytramConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Before heading to the details of the package I have some general comments on the basic quality of the software: please consider putting an installation section in the README, e.g https://github.com/Roger-luo/Configurations.jl#installation and please also set up some basic things for the above package as the following:
|
@Roger-luo thank you for your help! I just addressed your comments. In particular:
|
Hey @kungfugo, the example shown in the documentation doesn't work for me. However, it works if I call |
Also, the first phrase of the paper's summary bugs me a little.
|
Hey @meandmytram, have you pulled the latest changes? The source code only contains the function |
@meandmytram thank you very much for your comments! I will adjust the paper accordingly soon. |
I installed the package via |
This is my mistake. The latest version has not been tagged correctly. I will fix this asap. In the meanwhile you could pull from GitHub. There were no functional changes, only CI, tests and styling. Sorry for this problem. |
@meandmytram I released a new version (v0.1.2) compatible with the documentation and including your suggestions for the paper. |
@kungfugo Thanks for the changes, everything now seems to work according to documentation. One last but not the least thing I am a bit confused about is what it is exactly you're doing in the example. There is a lot of terminology I am not familiar with, sorry. From my understanding, you are building something like an ensemble of states from which you are then sampling and then analysing the entanglement, is this right? If that's something not too hard, could you please add a bit more explanations so that the content is more accessible. |
@meandmytram , thanks for your comment. I agree. |
Hi @kungfugo I have some minor comments on the APIs and manual, mode selection it might be better and more consistent with other Julia packages to just use myCoordStates = uniform_bell_sampler(100, d, "enclosurePolytope") can be myCoordStates = uniform_bell_sampler(100, d, :enclosurePolytope) it is not a big difference tho, but you can see this discussion to understand why
I think it is better to make the following example in the manual a self-contained example, the current one seems a bit hand-waving - one cannot directly run that code block without defining myExtendedKernel = extend_vpolytope_by_densitystates(tovrep(mySepKernel), newSepDensityStates) A similar issue exists for myOptimizedEWs = create_random_bounded_ews(
d,
myBasis,
n,
true,
50
) I'd suggest you to go through these examples and making sure clicking copy-paste buttons gives runnable scripts instead of just a code block. In summary, I think the functionality is solid, it does what is described. And a non-blocking suggestion from a user perspective, the current APIs are too long, and some of them seems to have too much positional arguments, e.g f(x) = analyse_coordstate(
d,
x,
myAnaSpec,
myBasis,
mySepKernel,
myWeylOperatorBasis,
myBasisDict,
missing,
myOptimizedCoodEWs
) or myAnaSpec = AnalysisSpecification(
true,
true,
true,
true,
true,
false,
true,
false
) If you need to specify configuration/options, I'd suggest checking out https://github.com/Roger-luo/Configurations.jl which is designed for the case. But if you are able to make it more automatic, or incremental (instead of putting a bunch of |
The example looks way less opaque now, I think once we're done with Roger's comments we'd be all set. |
Hi @Roger-luo,
Concerning your comments regarding Concerning your comments regarding the API: |
Done! Archive is now 10.5281/zenodo.7575767 |
@editorialbot set v0.1.5 as version |
Done! version is now v0.1.5 |
@editorialbot recommend-accept |
|
|
👋 @openjournals/pe-eics, this paper is ready to be accepted and published. Check final proof 👉📄 Download article If the paper PDF and the deposit XML files look good in openjournals/joss-papers#3909, then you can now move forward with accepting the submission by compiling again with the command |
OK! Looking good. That's me, an editor in chief should be along shortly for final checks. Many thanks @Roger-luo and @meandmytram for such detailed and interactive reviews, thank you @kungfugo for engaging with the process and being so responsive. (And my apologies for not noticing the reviews had finished 3 weeks ago!) |
@jarvist , @Roger-luo , @meandmytram thank you for your efforts! First time publishing with JOSS and it was a nice ecperience. Looking forward to the next ones. |
Hi @kungfugo, just doing some final checks before accepting.
|
Hi @kyleniemeyer , thank you for your comments. I have a released a new version v0.1.6 including your requests and registered it with zenodo. In particular:
|
@editorialbot generate pdf |
@kungfugo looks good, thank you! |
@editorialbot accept |
|
🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦 |
🐘🐘🐘 👉 Toot for this paper 👈 🐘🐘🐘 |
🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨 Here's what you must now do:
Any issues? Notify your editorial technical team... |
Congratulations @kungfugo on your article's publication in JOSS! Many thanks to @meandmytram and @Roger-luo for reviewing this submission, and @jarvist for editing. |
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippets:
This is how it will look in your documentation: We need your help! The Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:
|
(I'm going to reopen because it looks like the PDF is not yet appearing—I saw there were some systemic issues with GitHub Actions earlier.) |
Should be fixed now! |
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippets:
This is how it will look in your documentation: We need your help! The Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:
|
Submitting author: @kungfugo (Christopher Popp)
Repository: https://github.com/kungfugo/BellDiagonalQudits.jl
Branch with paper.md (empty if default branch):
Version: v0.1.5
Editor: @jarvist
Reviewers: @meandmytram, @Roger-luo
Archive: 10.5281/zenodo.7575767
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
@meandmytram & @Roger-luo, 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 @jarvist 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 @Roger-luo
📝 Checklist for @meandmytram
The text was updated successfully, but these errors were encountered: