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

My comments on paper.md #25

Closed
keunwoochoi opened this issue Jun 19, 2021 · 5 comments
Closed

My comments on paper.md #25

keunwoochoi opened this issue Jun 19, 2021 · 5 comments

Comments

@keunwoochoi
Copy link

Hi, I'm one of the JOSS reviewers of this repo. Before checking the software side of the work, I'd like to leave my comments on the paper here. I read this version (openjournals/joss-reviews#3218 (comment)) of the paper. I noticed it's a bit updated but not the part I'd like to mention about.


L11-14: It's a bit long, making the sentence slightly vague.

L25-26: What do you mean exactly by "multifaceted nature"? This can be clarified.

L29: "previous solution" -> "previous solutions". Also, maybe some citations of the solutions?

L30-31: I think this sentences can be re-written a bit to clarify. The current one confused me a bit.

L34-35: I found this sentence out of context a little. Wouldn't it make more sense to mention it in the first paragraph of "Statement of need"?

L36-37: -> "..original papers. The models can.."

L56-58: Really? But how much instrument-agnostic is it?

L61-62: "which predicts percussive events from a given input audio." → probably no need to mention? This is assumed in the definition of drum transcription.

L69-70: It sounds like the pitch extractor not a "module" while note segmentation is, and that confuses me.

L76 : "The training includes.." → "The training data includes.."

L80: "function" → probably, keep using "model" for consistency?

L109: remove "also".


Best,
Keunwoo

@faroit
Copy link
Contributor

faroit commented Jun 21, 2021

just for the referencing: the review issue is here: openjournals/joss-reviews#3391

@keunwoochoi
Copy link
Author

just for the referencing: the review issue is here: openjournals/joss-reviews#3347

oops, i think @faroit you meant "openjournals/joss-reviews#3391" this. but you mean, comments like this are supposed to be communicated at the review thread?

@faroit
Copy link
Contributor

faroit commented Jun 21, 2021

@keunwoochoi yes, sure, sorry. copy-pasted the wrong link....

but you mean, comments like this are supposed to be communicated at the review thread?

No, its exactly how it's supposed to be 👍
I was just linking to the review issue, since you started reviewing before the official review even started ;-)

@keunwoochoi
Copy link
Author

(Pinging @BreezeWhite in case you missed it)

@BreezeWhite
Copy link
Member

Hi, sorry for the late update.It's been busy these days.

For issue 2-5: We decide to rewrite the whole "Statement of need" section as can been seen in the updated version.

For L56-58: We understand your concern. Following the definition in the original paper, the term “instrument agnostic” means that the instrument classes in the input can be “uninformed” and we truly did not mean that the model can predict “unseen” instrument classes. To avoid misleading, we guide the readers to read the reference paper

For L69-L70: The system for vocal transcription contains a pitch extraction and a note segmentation module.

Thanks for pointing out the above issues. Please check if there are still needs to modify.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants