openjournals / joss-reviews

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

[REVIEW]: pytorch-widedeep: A flexible package for multimodal-deep-learning #5027

Closed editorialbot closed 1 year ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@5uperpalo<!--end-author-handle-- (Pavol Mulinka) Repository: https://github.com/jrzaurin/pytorch-widedeep Branch with paper.md (empty if default branch): joss_paper Version: 1.2.0 Editor: !--editor-->@osorensen<!--end-editor-- Reviewers: @siboehm, @makoeppel Archive: 10.5281/zenodo.7908172

Status

status

Status badge code:

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

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

@siboehm & @dataplayer12 & @makoeppel, 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:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @osorensen 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 @makoeppel

πŸ“ Checklist for @siboehm

osorensen commented 1 year ago

@5uperpalo, I'm now reading through the paper and will post editorial comments as issues in the source repo.

editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.48550/ARXIV.2003.06505 is OK
- 10.48550/ARXIV.2108.09084 is OK
- 10.48550/arXiv.1606.07792 is OK
- 10.18653/v1/N16-1174 is OK
- 10.1109/cvpr.2016.90 is OK
- 10.1109/CVPR.2018.00716 is OK
- 10.1109/CVPR.2017.634 is OK
- 10.5244/c.30.87 is OK
- 10.1109/TNNLS.2022.3158966 is OK
- 10.1109/CVPR.2019.00293 is OK
- 10.1162/neco.1997.9.8.1735 is OK
- 10.3115/v1/w14-4012 is OK
- 10.1163/1574-9347_dnp_e612900 is OK
- 10.48550/arXiv.1708.05123 is OK

MISSING DOIs

- None

INVALID DOIs

- None
5uperpalo commented 1 year ago

@editorialbot generate pdf

5uperpalo commented 1 year ago

@editorialbot check references

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.48550/ARXIV.2003.06505 is OK
- 10.48550/ARXIV.2108.09084 is OK
- 10.48550/arXiv.1606.07792 is OK
- 10.18653/v1/N16-1174 is OK
- 10.1109/cvpr.2016.90 is OK
- 10.1109/CVPR.2018.00716 is OK
- 10.1109/CVPR.2017.634 is OK
- 10.5244/c.30.87 is OK
- 10.1109/TNNLS.2022.3158966 is OK
- 10.1109/CVPR.2019.00293 is OK
- 10.1162/neco.1997.9.8.1735 is OK
- 10.3115/v1/w14-4012 is OK
- 10.48550/arXiv.1708.05123 is OK

MISSING DOIs

- None

INVALID DOIs

- None
5uperpalo commented 1 year ago

@editorialbot generate pdf

5uperpalo commented 1 year ago

@editorialbot check references

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

5uperpalo commented 1 year ago

@osorensen thank you for your comments, I fixed all the issues you pointed out, I am also sorry for the delay, but it was a holiday season - which means a lot of additional work while colleagues are gone for vacations :) :P ; please look at the the fixes and let me know if you have any other comments

editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.48550/ARXIV.2003.06505 is OK
- 10.48550/ARXIV.2108.09084 is OK
- 10.48550/arXiv.1606.07792 is OK
- 10.18653/v1/N16-1174 is OK
- 10.1109/cvpr.2016.90 is OK
- 10.1109/CVPR.2018.00716 is OK
- 10.1109/CVPR.2017.634 is OK
- 10.5244/c.30.87 is OK
- 10.1109/TNNLS.2022.3158966 is OK
- 10.1109/CVPR.2019.00293 is OK
- 10.1162/neco.1997.9.8.1735 is OK
- 10.3115/v1/w14-4012 is OK
- 10.48550/arXiv.1708.05123 is OK

MISSING DOIs

- None

INVALID DOIs

- None
osorensen commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

5uperpalo commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

5uperpalo commented 1 year ago

@osorensen again, thank you very much for highlighting all the issues, I fixed all of them and closed the GitHub issues, please let me know if everything is ok ... again thank you for your time!!

osorensen commented 1 year ago

Thanks @5uperpalo!

At this point could you:

I can then move forward with recommending acceptance of the submission.

osorensen commented 1 year ago

@5uperpalo, how is it going with the points mentioned in my previous post?

jrzaurin commented 1 year ago

This is the link to the Zenodo archive publication.

https://doi.org/10.5281/zenodo.7908172

This is the link to the tag of the release:

https://github.com/jrzaurin/pytorch-widedeep/tree/joss_paper_finalised

which is connected to our joss_paper branch

Let us know if you need anything else, please

and thanks for doing all this

osorensen commented 1 year ago

Thanks @jrzaurin. Can you please change the title of the Zenodo archive so it matches the paper title exactly? That is, change from "jrzaurin/pytorch-widedeep: Zenodo Release" to "pytorch-widedeep: A flexible package for multimodal-deep-learning".

jrzaurin commented 1 year ago

hey @osorensen. Thanks for your answer.

The title of the release has been updated and I guess this will sync with the Zenodo archive. Otherwise we will have to publish a new release (?)

(Cc: @5uperpalo )

osorensen commented 1 year ago

Thanks @jrzaurin. The Zenodo archive did not sync, so I think you'll have to create a new one.

jrzaurin commented 1 year ago

hey @osorensen

thanks for letting me know. I will do so tomorrow

osorensen commented 1 year ago

Any update on this @jrzaurin?

jrzaurin commented 1 year ago

Hey, I tried to simply create a new release and I got a citations error:

Screenshot 2023-05-18 at 11 24 31 AM

Whenever I have a sec we will try to fix it

jrzaurin commented 1 year ago

Now we have this error:

{
    "errors": "The license ID you have selected is not present in our system. For the available licenses please check in the following URL https://developers.zenodo.org/#licenses"
}

Publishing here is proving to be a challenge. We'll do our best if time permits.

osorensen commented 1 year ago

Now we have this error:

{
    "errors": "The license ID you have selected is not present in our system. For the available licenses please check in the following URL https://developers.zenodo.org/#licenses"
}

Publishing here is proving to be a challenge. We'll do our best if time permits.

Strange that you get all these errors when creating a Zenodo archive. @openjournals/dev, could you please help?

xuanxu commented 1 year ago

I'd recommend to update the metadata directly in the Zenodo page, once logged in there is an Edit button that allows to update title and author info.

osorensen commented 1 year ago

@jrzaurin or @5uperpalo, could you please try what @xuanxu suggests in the post above?

osorensen commented 1 year ago

πŸ‘‹ @jrzaurin or @5uperpalo, could you please try what @xuanxu suggests in the post above?

This is the last step before I can recommend the paper for acceptance.

osorensen commented 1 year ago

πŸ‘‹ @jrzaurin or @5uperpalo, could you please try what @xuanxu suggests in the post above?

This is the last step before I can recommend the paper for acceptance.

jrzaurin commented 1 year ago

@5uperpalo could you please take this task? please please πŸ™‚

jrzaurin commented 1 year ago

@osorensen

hey, we went to the page that @xuanxu suggested and edit it and publish it: https://zenodo.org/record/7908172

Let me know if this is enough

Cheers! Javier

osorensen commented 1 year ago

Thanks @jrzaurin. This looks sufficient.

osorensen commented 1 year ago

@jrzaurin, what is the exact version of pytorch-widedeep at https://zenodo.org/record/7908172? Is it v1.2.1, or something else?

jrzaurin commented 1 year ago

is actually 1.2.0,

there is an inconsistency somewhere right?

osorensen commented 1 year ago

is actually 1.2.0,

there is an inconsistency somewhere right?

@jrzaurin, the important point is that the Zenodo archive contains the most recent version of the software, with all changes made during the review process included. Does 1.2.0 include this?

jrzaurin commented 1 year ago

Ok, then not really no, we are releasing stuff as we can, for example we merged a PR today, and we are about to merge a new PR soon.

Should we then wait for it? Maybe is better if we publish just after merging that PR πŸ€·πŸ»β€β™‚οΈ

osorensen commented 1 year ago

As long as the release is up-to-date after the reviewers finished their reviews, it does not have to be the very last release. It seems to be that no changes to the software were made during the review. Is that correct? If so, I suggest we go ahead with version 1.2.0.

jrzaurin commented 1 year ago

Sounds perfect for me

osorensen commented 1 year ago

@editorialbot set 1.2.0 as version

editorialbot commented 1 year ago

Done! version is now 1.2.0

osorensen commented 1 year ago

@editorialbot set 10.5281/zenodo.7908172 as archive

editorialbot commented 1 year ago

Done! archive is now 10.5281/zenodo.7908172

osorensen commented 1 year ago

@editorialbot recommend-accept

editorialbot commented 1 year ago
Attempting dry run of processing paper acceptance...
editorialbot commented 1 year ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.48550/ARXIV.2003.06505 is OK
- 10.48550/ARXIV.2108.09084 is OK
- 10.48550/arXiv.1606.07792 is OK
- 10.18653/v1/N16-1174 is OK
- 10.1109/cvpr.2016.90 is OK
- 10.1109/CVPR.2018.00716 is OK
- 10.1109/CVPR.2017.634 is OK
- 10.5244/c.30.87 is OK
- 10.1109/TNNLS.2022.3158966 is OK
- 10.1109/CVPR.2019.00293 is OK
- 10.1162/neco.1997.9.8.1735 is OK
- 10.3115/v1/w14-4012 is OK
- 10.3390/info11020108 is OK
- 10.48550/arXiv.1708.05123 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 1 year ago

:wave: @openjournals/dsais-eics, this paper is ready to be accepted and published.

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/4317, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

gkthiruvathukal commented 1 year ago

@editorialbot accept

editorialbot commented 1 year ago
Doing it live! Attempting automated processing of paper acceptance...