openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
719 stars 38 forks source link

[REVIEW]: scikit-hubness: Hubness Reduction and Approximate Neighbor Search #1957

Closed whedon closed 4 years ago

whedon commented 4 years ago

Submitting author: @VarIr (Roman Feldbauer) Repository: https://github.com/VarIr/scikit-hubness/ Version: v0.21.2 Editor: @terrytangyuan Reviewer: @ryEllison, @aozorahime Archive: 10.5281/zenodo.3607202

Status

status

Status badge code:

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

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

@ryEllison & @aozorahime, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @terrytangyuan know.

Please try and complete your review in the next two weeks

Review checklist for @ryEllison

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @aozorahime

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 4 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @ryEllison, @aozorahime 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/openjournals/joss-reviews) 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:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf
whedon commented 4 years ago
Attempting to check references...
whedon commented 4 years ago
Attempting PDF compilation. Reticulating splines etc...
whedon commented 4 years ago

OK DOIs

- 10.1109/TKDE.2013.25 is OK
- 10.1016/j.knosys.2015.06.010 is OK

MISSING DOIs

- https://doi.org/10.1109/icdmw.2013.101 may be missing for title: Can Shared Nearest Neighbors Reduce Hubness in High-Dimensional Spaces?
- https://doi.org/10.1080/09298215.2017.1354891 may be missing for title: Mutual proximity graphs for improved reachability in music recommendation
- https://doi.org/10.1007/s10115-018-1205-y may be missing for title: A comprehensive empirical comparison of hubness reduction in high-dimensional spaces
- https://doi.org/10.1109/icbk.2018.00055 may be missing for title: Fast Approximate Hubness Reduction for Large High-Dimensional Data
- https://doi.org/10.1109/tpami.2018.2889473 may be missing for title: Efficient and robust approximate nearest neighbor search using Hierarchical Navigable Small World graphs
- https://doi.org/10.1109/ijcnn.2015.7280303 may be missing for title: The unbalancing effect of hubs on K-medoids clustering in high-dimensional spaces

INVALID DOIs

- None
whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

terrytangyuan commented 4 years ago

@VarIr Looks like there are some missing DOIs. Could you fix it?

VarIr commented 4 years ago

Thanks for reminding me! I'll look to it asap.

VarIr commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

VarIr commented 4 years ago

@whedon commands

whedon commented 4 years ago

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

# Ask Whedon to check repository statistics for the submitted software
@whedon check repository
VarIr commented 4 years ago

@whedon check references

whedon commented 4 years ago
Reference check summary:

OK DOIs

- 10.1109/ICDMW.2013.101 is OK
- 10.1080/09298215.2017.1354891 is OK
- 10.1007/s10115-018-1205-y is OK
- 10.1109/ICBK.2018.00055 is OK
- 10.1109/TKDE.2013.25 is OK
- 10.1109/IJCNN.2015.7280303 is OK
- 10.4230/LIPIcs.ESA.2019.10 is OK

MISSING DOIs

- https://doi.org/10.1109/tpami.2018.2889473 may be missing for title: Efficient and robust approximate nearest neighbor search using Hierarchical Navigable Small World graphs

INVALID DOIs

- None
VarIr commented 4 years ago

@terrytangyuan The missing DOI is for an article from arXiv, which does not provide DOIs, afaik. What should I do about this?

arfon commented 4 years ago

@terrytangyuan The missing DOI is for an article from arXiv, which does not provide DOIs, afaik. What should I do about this?

@VarIr - perhaps you can update your reference to what looks to be the (journal) published version of the same article. This way, the authors will receive citation credit from you. BTW, this comment assumes that the arXiv version is the same paper as https://doi.org/10.1109/tpami.2018.2889473 which during my very brief inspection appears to be the case.

VarIr commented 4 years ago

Thank you for the hint! Indeed, I missed that it was eventually published in a journal.

VarIr commented 4 years ago

@whedon check references

whedon commented 4 years ago
Reference check summary:

OK DOIs

- 10.1109/ICDMW.2013.101 is OK
- 10.1080/09298215.2017.1354891 is OK
- 10.1007/s10115-018-1205-y is OK
- 10.1109/ICBK.2018.00055 is OK
- 10.1109/TKDE.2013.25 is OK
- 10.1109/IJCNN.2015.7280303 is OK
- 10.4230/LIPIcs.ESA.2019.10 is OK

MISSING DOIs

- https://doi.org/10.1109/tpami.2018.2889473 may be missing for title: Efficient and robust approximate nearest neighbor search using Hierarchical Navigable Small World graphs

INVALID DOIs

- None
VarIr commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

VarIr commented 4 years ago

@whedon check references

whedon commented 4 years ago
Reference check summary:

OK DOIs

- 10.1109/ICDMW.2013.101 is OK
- 10.1080/09298215.2017.1354891 is OK
- 10.1007/s10115-018-1205-y is OK
- 10.1109/ICBK.2018.00055 is OK
- 10.1109/TPAMI.2018.2889473 is OK
- 10.1109/TKDE.2013.25 is OK
- 10.1109/IJCNN.2015.7280303 is OK
- 10.4230/LIPIcs.ESA.2019.10 is OK

MISSING DOIs

- None

INVALID DOIs

- None
ryEllison commented 4 years ago

I'm satisfied that the requirements for publication have been met, and as such, I'm happy to see this accepted and published as submitted.

The documentation is extensive and functionality is exemplified both in text and through example usages. In an odd confluence of events, my lab has recently advanced a technique for time-series data, specifically in neurobiological sciences, related to some of the fundamental concepts of this work.

As trivial as it may seem, I thought it prudent to bring something to the authors' attention. Upon installation, the annoy dependency threw an error and exited because of it's need for Visual Studio C++ Build Tools. Now, admittedly, this wouldn't have occurred on my office or lab computers. However, I tested this package on a new laptop that I had yet to install C++ Build Tools for until my attempted installation of this package brought its absence to my attention. I suppose its need is handled in the error message of annoy, as I said, it may seem trivial, but I thought it best to bring even trivialities to your attention.

Congrats!

aozorahime commented 4 years ago

I probably don't give any comment since the quality of the paper and the novelty are both good. and I learned a lot from your paper. Congrats and good luck for next findings :)

VarIr commented 4 years ago

Thank you both @ryEllison and @aozorahime for reviewing my work and your positive feedback!

@ryEllison Do you mind pointing me to a paper of yours regarding the technique you mentioned? I'm interested, because I've got some neuroscience background and, to add to the odd events, just recently started a small project about neurophysiological time series. Also thanks for bringing the annoy issue to my attention. I'm mostly dealing with Linux or Mac, so Windows issues slip through more easily. Could you perhaps open an issue at https://github.com/VarIr/scikit-hubness/ and provide the error message?

VarIr commented 4 years ago

@aozorahime Could you please tick off the box for Performance, if you think this is fulfilled? I think only then the review is completed.

aozorahime commented 4 years ago

done :+1: i am sorry if i made it delay

terrytangyuan commented 4 years ago

@whedon check references

whedon commented 4 years ago
Reference check summary:

OK DOIs

- 10.1109/ICDMW.2013.101 is OK
- 10.1080/09298215.2017.1354891 is OK
- 10.1007/s10115-018-1205-y is OK
- 10.1109/ICBK.2018.00055 is OK
- 10.1109/TPAMI.2018.2889473 is OK
- 10.1109/TKDE.2013.25 is OK
- 10.1109/IJCNN.2015.7280303 is OK
- 10.4230/LIPIcs.ESA.2019.10 is OK

MISSING DOIs

- None

INVALID DOIs

- None
terrytangyuan commented 4 years ago

Thanks everyone! @VarIr At this point could you make a new release of this software that includes the changes that have resulted from this review. Then, please make an archive of the software in Zenodo/figshare/other service and update this thread with the DOI of the archive? For the Zenodo/figshare archive, please make sure that:

VarIr commented 4 years ago

@terrytangyuan I've just created a zenodo archive at https://zenodo.org/record/3607202 and DOI 10.5281/zenodo.3607202. The new version number is v0.21.2.

terrytangyuan commented 4 years ago

@whedon set v0.21.2 as version

whedon commented 4 years ago

OK. v0.21.2 is the version.

terrytangyuan commented 4 years ago

@whedon set 10.5281/zenodo.3607202 as archive

whedon commented 4 years ago

OK. 10.5281/zenodo.3607202 is the archive.

terrytangyuan commented 4 years ago

@whedon accept

whedon commented 4 years ago
Attempting dry run of processing paper acceptance...
whedon commented 4 years ago
Reference check summary:

OK DOIs

- 10.1109/ICDMW.2013.101 is OK
- 10.1080/09298215.2017.1354891 is OK
- 10.1007/s10115-018-1205-y is OK
- 10.1109/ICBK.2018.00055 is OK
- 10.1109/TPAMI.2018.2889473 is OK
- 10.1109/TKDE.2013.25 is OK
- 10.1109/IJCNN.2015.7280303 is OK
- 10.4230/LIPIcs.ESA.2019.10 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 4 years ago

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/1213

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/1213, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true
terrytangyuan commented 4 years ago

@openjournals/joss-eics This paper looks good to me. Could you take it from here?

kyleniemeyer commented 4 years ago

Hi @VarIr, the article looks good to me overall, with just a few minor things that could be corrected:

ryEllison commented 4 years ago

@VarIr paper is currently under review. Once it's sharable, I don't mind to send it along. Also, certainly interested in hearing about what you're working on regarding neurophysiological time-series data. My email is ryan.dean.ellison@gmail.com. Perhaps we should move the conversation there...

VarIr commented 4 years ago

@whedon generate pdf from branch develop

whedon commented 4 years ago
Attempting PDF compilation from custom branch develop. Reticulating splines etc...
whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

VarIr commented 4 years ago

@kyleniemeyer Thanks for pointing that out. I removed the non-essential links altogether, and embedded the links to the github repo and docs into the text body. Em-dashes now come w/o surrounding spaces.

kyleniemeyer commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

:point_right: Check article proof :page_facing_up: :point_left:

kyleniemeyer commented 4 years ago

@whedon accept

whedon commented 4 years ago
Attempting dry run of processing paper acceptance...