Closed whedon closed 3 years ago
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @christianpeel, @mcognetta it looks like you're currently assigned to review this paper :tada:.
:star: Important :star:
If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/JuliaCon/proceedings-review) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿
To fix this do the following two things:
For a list of things I can do to help you, just type:
@whedon commands
Attempting PDF compilation. Reticulating splines etc...
@whedon check references
Attempting to check references...
OK DOIs
- 10.1016/S0377-0427(00)00393-9 is OK
MISSING DOIs
- https://doi.org/10.1162/neco_a_01096 may be missing for title: A comparative study of pairwise learning methods based on kernel ridge regression
- https://doi.org/10.1109/tnnls.2017.2727545 may be missing for title: Fast Kronecker product kernel methods via generalized vec trick
INVALID DOIs
- 10.1145/1756006.1756039 is INVALID
Are added to the ref.bib
, but these don't appear in the paper, as dictated by the style file.
@whedon check references
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
I recommend that the paper be accepted as-is.
@whedon commands
Here are some things you can ask me to do:
# List Whedon's capabilities
@whedon commands
# List of editor GitHub usernames
@whedon list editors
# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers
EDITORIAL TASKS
# Compile the paper
@whedon generate pdf
# Compile the paper from alternative branch
@whedon generate pdf from branch custom-branch-name
# Ask Whedon to check the references for missing DOIs
@whedon check references
Recommendation: accept with minor modifications.
Minor paper comments:
To fully make use the power of Julia, we will explore three directions.
Should be '...make use of the...'
Minor code comments:
show
method should be improved to be similar to printing large dense matrices. Currently it does nothing.collect
must be called. It makes sense to have a Matrix
constructor as well.where
statements when the type parameter is unused. For example:
https://github.com/MichielStock/Kronecker.jl/blob/a1d9d068555ff74cf99771bdbc68b71e8a90a7be/src/base.jl#L54
https://github.com/MichielStock/Kronecker.jl/blob/a1d9d068555ff74cf99771bdbc68b71e8a90a7be/src/base.jl#L77Thank you for all the feedback and suggestions. I updated the code today, providing better integration with the AbstractArray
interface. I will update the documentation and push to master branch.
@MichielStock can you let us know when you have done the changes?
I have done the changes and merged them into the master
branch.
I very much like the plot of benchmark times that you added to the Readme.
ping @mcognetta to continue the review
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
I will play around with the functionality today but otherwise it looks good now. I really like the logo and the new expanded readme.
@mcognetta how are you doing on the review here?
In light of the revisions, I am changing my recommendation to accept as-is.
@MichielStock a couple additional comments: Section 1: "In linear algebra" -> is it necessary, given that the context is operations over matrices?
"gives rises to some elegant mathematics", elegant is a bit odd to see (hard to define). To some properties of interest?
"The elementary functions of LinearAlgebra
" -> "The elementary functions of the LinearAlgebra
module from the Julia standard library".
I would indicate more clearly that "vec trick" is a standard term in the literate, by using quotes and hints like "so-called vec trick" with the reference to [5]. I would not use "tricks from linear algebra" in the abstract as it sounds a bit informal.
Could you cite the packages you are referencing by adding a bibtex entry? (Zygote, CuArrays), also: CuArrays -> CuArrays.jl
These comments are now done as of 497ae84
On 3 Sep 2019, at 17:05, Mathieu Besançon notifications@github.com wrote:
@MichielStock https://github.com/MichielStock a couple additional comments: Section 1: "In linear algebra" -> is it necessary, given that the context is operations over matrices?
"gives rises to some elegant mathematics", elegant is a bit odd to see (hard to define). To some properties of interest?
"The elementary functions of LinearAlgebra" -> "The elementary functions of the LinearAlgebra module from the Julia standard library".
I would indicate more clearly that "vec trick" is a standard term in the literate, by using quotes and hints like "so-called vec trick" with the reference to [5]. I would not use "tricks from linear algebra" in the abstract as it sounds a bit informal.
Prospects Could you cite the packages you are referencing by adding a bibtex entry? (Zygote, CuArrays), also: CuArrays -> CuArrays.jl
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JuliaCon/proceedings-review/issues/15?email_source=notifications&email_token=ABN5PI4YP5J4F6CPYUOFCITQHZ4J7A5CNFSM4H3AVCF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5YQHBI#issuecomment-527500165, or mute the thread https://github.com/notifications/unsubscribe-auth/ABN5PI726IOPEQS7KMY6FH3QHZ4J7ANCNFSM4H3AVCFQ.
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
Alternatively, one can make use of Julia's Unicode support:
K = A \cross B
Can you make K = A cross B an equation as to have it all on the same line?
overloaded to work with the
Kronecker
type.
As I understand, there are several Kronecker
types, should it be pluralized?
The last sentence of section 2 feels like a conclusion and prospect, should it go at the beginning of section 3?
At the end of section 3, could you add some more details on the extensions for high-order Kronecker products? (not a big deal if the text is a bit above 1 page)
Updated. Also added a section on higher-order Kronecker products, which are now supported.
On 5 Sep 2019, at 14:52, Mathieu Besançon notifications@github.com wrote:
Alternatively, one can make use of Julia's Unicode support: K = A \cross B
Can you make K = A cross B an equation as to have it all on the same line?
overloaded to work with the Kronecker type.
As I understand, there are several Kronecker types, should it be pluralized?
The last sentence of section 2 feels like a conclusion and prospect, should it go at the beginning of section 3?
At the end of section 3, could you add some more details on the extensions for high-order Kronecker products? (not a big deal if the text is a bit above 1 page)
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JuliaCon/proceedings-review/issues/15?email_source=notifications&email_token=ABN5PIYOMVVB6AKRB2YFJDTQID6KLA5CNFSM4H3AVCF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD567NZY#issuecomment-528348903, or mute the thread https://github.com/notifications/unsubscribe-auth/ABN5PI72H7G2MKEGIJA7C2LQID6KLANCNFSM4H3AVCFQ.
Still the code snippet to turn into equation:
Alternatively, one can make use of Julia's Unicode support: K = A \cross B
Can you make K = A cross B an equation as to have it all on the same line?
Done in af1d3ced https://github.com/MichielStock/Kronecker.jl/commit/af1d3cedb2e58902eead8584f3e04e9ddcab5ea1. Would be nice to wrap this up.
On 19 Sep 2019, at 16:40, Mathieu Besançon notifications@github.com wrote:
Still the code snippet to turn into equation:
Alternatively, one can make use of Julia's Unicode support: K = A \cross B
Can you make K = A cross B an equation as to have it all on the same line?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JuliaCon/proceedings-review/issues/15?email_source=notifications&email_token=ABN5PIZLBQ4KFMNQRKMGPXLQKOFMHA5CNFSM4H3AVCF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7DWR2I#issuecomment-533162217, or mute the thread https://github.com/notifications/unsubscribe-auth/ABN5PIZG6HXJQA246U2RN5DQKOFMHANCNFSM4H3AVCFQ.
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
Looks good here
@whedon accept
No archive DOI set. Exiting...
ah right. @MichielStock the next steps are: archiving the code repo with Zenodo to obtain a DOI, we'll then be able to attach the DOI to the paper
Registered to Zenodo and a badge is added to the readme.md
:
On 27 Sep 2019, at 19:13, Mathieu Besançon notifications@github.com wrote:
ah right. @MichielStock https://github.com/MichielStock the next steps are: archiving the code repo with Zenodo to obtain a DOI, we'll then be able to attach the DOI to the paper
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JuliaCon/proceedings-review/issues/15?email_source=notifications&email_token=ABN5PIZDWAY77AZQTPYJWQLQLY5L5A5CNFSM4H3AVCF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7ZRJ4I#issuecomment-536024305, or mute the thread https://github.com/notifications/unsubscribe-auth/ABN5PI7N772HZRUAELXR2KTQLY5L5ANCNFSM4H3AVCFQ.
@whedon commands
Here are some things you can ask me to do:
# List all of Whedon's capabilities
@whedon commands
# Assign a GitHub user as the sole reviewer of this submission
@whedon assign @username as reviewer
# Add a GitHub user to the reviewers of this submission
@whedon add @username as reviewer
# Remove a GitHub user from the reviewers of this submission
@whedon remove @username as reviewer
# List of editor GitHub usernames
@whedon list editors
# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers
# Change editorial assignment
@whedon assign @username as editor
# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive
# Set the software version at the top of the issue e.g.
@whedon set v1.0.1 as version
# Open the review issue
@whedon start review
EDITORIAL TASKS
# Compile the paper
@whedon generate pdf
# Compile the paper from alternative branch
@whedon generate pdf from branch custom-branch-name
# Remind an author or reviewer to return to a review after a
# certain period of time (supported units days and weeks)
@whedon remind @reviewer in 2 weeks
# Ask Whedon to accept the paper and deposit with Crossref
@whedon accept
# Ask Whedon to check the references for missing DOIs
@whedon check references
@whedon set 10.5281/zenodo.3463941 as archive
OK. 10.5281/zenodo.3463941 is the archive.
@whedon set v0.3.0 as version
OK. v0.3.0 is the version.
@whedon accept
Attempting dry run of processing paper acceptance...
Submitting author: !--author-handle-->@michielstock<!--end-author-handle-- (Michiel Stock) Repository: https://github.com/michielStock/Kronecker.jl Branch with paper.md (empty if default branch): Version: Editor: !--editor-->@matbesancon<!--end-editor-- Reviewers: @christianpeel, @mcognetta Archive:
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
@christianpeel & @mcognetta, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
The reviewer guidelines are available here: https://proceedings.juliacon.org/guide/reviewers. Any questions/concerns please let @matbesancon know.
✨ Please try and complete your review in the next two weeks ✨
Review checklist for @christianpeel
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Paper format
paper.tex
file include a list of authors with their affiliations?Content
Review checklist for @mcognetta
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Paper format
paper.tex
file include a list of authors with their affiliations?Content