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

[PRE REVIEW]: Kinetic.jl: A lightweight finite volume toolbox in Julia #2942

Closed
whedon opened this issue Jan 6, 2021 · 69 comments
Closed

[PRE REVIEW]: Kinetic.jl: A lightweight finite volume toolbox in Julia #2942

whedon opened this issue Jan 6, 2021 · 69 comments

Comments

@whedon
Copy link

whedon commented Jan 6, 2021

Submitting author: @vavrines (Tianbai Xiao)
Repository: https://github.com/vavrines/Kinetic.jl
Version: v0.7.0
Editor: @diehlpk
Reviewers: @rdeits, @jarvist
Managing EiC: Arfon Smith

⚠️ JOSS reduced service mode ⚠️

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Author instructions

Thanks for submitting your paper to JOSS @vavrines. Currently, there isn't an JOSS editor assigned to your paper.

The author's suggestion for the handling editor is @Kevin-Mattheus-Moerman.

@vavrines if you have any suggestions for potential reviewers then please mention them here in this thread (without tagging them with an @). In addition, this list of people have already agreed to review for JOSS and may be suitable for this submission (please start at the bottom of the list).

Editor instructions

The JOSS submission bot @whedon is here to help you find and assign reviewers and start the main review. To find out what @whedon can do for you type:

@whedon commands
@whedon
Copy link
Author

whedon commented Jan 6, 2021

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks.

⚠️ JOSS reduced service mode ⚠️

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

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
Copy link
Author

whedon commented Jan 6, 2021

Failed to discover a Statement of need section in paper

@whedon
Copy link
Author

whedon commented Jan 6, 2021

Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.11 s (322.1 files/s, 40919.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
TOML                             5            512              2           2405
Julia                           12            153            235            820
Markdown                        15             66              0            324
YAML                             4              5              7            116
TeX                              1              0              0             55
-------------------------------------------------------------------------------
SUM:                            37            736            244           3720
-------------------------------------------------------------------------------


Statistical information for the repository '7c6b994d8ff9880733838cee' was
gathered on 2021/01/06.
No commited files with the specified extensions were found.

@whedon
Copy link
Author

whedon commented Jan 6, 2021

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- None

MISSING DOIs

- 10.1137/141000671 may be a valid DOI for title: Julia: A fresh approach to numerical computing

INVALID DOIs

- https://doi.org/10.5281/zenodo.4025432 is INVALID because of 'https://doi.org/' prefix

@whedon
Copy link
Author

whedon commented Jan 6, 2021

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@arfon
Copy link
Member

arfon commented Jan 6, 2021

@whedon query scope

@vavrines thanks for your submission to JOSS – due to the relatively small size of this code, the editors will now discuss if it meets the substantial scholarly effort criterion for review by JOSS. We should get back to you sometime next week. If you want to fix the DOIs (noting that whedon's suggestions are not always right), you can, then use the following commands (one at a time, as the first line of a new comment) to regenerate the PDF and check the references.

@whedon generate pdf
@whedon check references

@whedon
Copy link
Author

whedon commented Jan 6, 2021

Submission flagged for editorial review.

@whedon whedon added the query-scope Submissions of uncertain scope for JOSS label Jan 6, 2021
@vavrines
Copy link

vavrines commented Jan 6, 2021

@whedon query scope

@vavrines thanks for your submission to JOSS – due to the relatively small size of this code, the editors will now discuss if it meets the substantial scholarly effort criterion for review by JOSS. We should get back to you sometime next week. If you want to fix the DOIs (noting that whedon's suggestions are not always right), you can, then use the following commands (one at a time, as the first line of a new comment) to regenerate the PDF and check the references.

@whedon generate pdf
@whedon check references

Thanks for the prompt support!
Maybe I should mention it here. The package is a combination that reexports KitBase, KitML and KitFort, and the total size of codes should be fine : )

@vavrines
Copy link

vavrines commented Jan 6, 2021

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Jan 6, 2021

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@arfon
Copy link
Member

arfon commented Jan 6, 2021

Maybe I should mention it here. The package is a combination that reexports KitBase, KitML and KitFort, and the total size of codes should be fine : )

@vavrines - I'm not sure I follow sorry. Are you saying that this package is actually four different codebases combined, or that KitBase, KitML, and KitFort are dependencies of this one?

If so, JOSS' scholarly effort criterion is applied to the package under review (Kinetic.jl) in this case, not the dependent projects. From my assessment (with the caveat that I'm not a Julia user), this submission looks to be ~800 lines of Julia code. Please correct me if I'm missing something here.

@vavrines
Copy link

vavrines commented Jan 6, 2021

Maybe I should mention it here. The package is a combination that reexports KitBase, KitML and KitFort, and the total size of codes should be fine : )

@vavrines - I'm not sure I follow sorry. Are you saying that this package is actually four different codebases combined, or that KitBase, KitML, and KitFort are dependencies of this one?

If so, JOSS' scholarly effort criterion is applied to the package under review (Kinetic.jl) in this case, not the dependent projects. From my assessment (with the caveat that I'm not a Julia user), this submission looks to be ~800 lines of Julia code. Please correct me if I'm missing something here.

@arfon Thanks for the clarification.
Yes they're the codebases. The structs and methods have been imported into this main package so there's no need to review these sub repositories.
It's a common strategy in Julia to reduce precompilation time, e.g. DIfferentialEquations.jl and Yao.jl

@dpsanders
Copy link

The sub repositories are an integral part of the Julia package and should definitely be reviewed.

@Kevin-Mattheus-Moerman
Copy link
Member

@vavrines can you clarify what the entire codebase is that should be considered/reviewed here? The package Kinetic.jl appears too minor on its own. If the core functionality provided requires KitBase, KitML and KitFort (and perhaps kineticpy), all of which you wrote to accompany Kinetic (if I understand this correctly), then these should perhaps all be reviewed altogether and seen as a single submission, can you confirm?

@vavrines
Copy link

vavrines commented Jan 18, 2021

@vavrines can you clarify what the entire codebase is that should be considered/reviewed here? The package Kinetic.jl appears too minor on its own. If the core functionality provided requires KitBase, KitML and KitFort (and perhaps kineticpy), all of which you wrote to accompany Kinetic (if I understand this correctly), then these should perhaps all be reviewed altogether and seen as a single submission, can you confirm?

Hi @Kevin-Mattheus-Moerman , thanks for the reply.
Yes I can herby confirm the core functionality is provided together with KitBase, KitML and KitFort, and they serve as a single submission.

Best, Tianbai

@jedbrown
Copy link
Member

If the capability has rough parity with Clawpack, then there's no question about it being within scope. I can't tell if that's the case -- the examples are surely much less developed and documented. From briefly browsing the docs, I don't see where to look for boundary conditions, the extent of its multi-dimensional capability, time integrators, etc. What is supported? How fast is it compared to mature implementations (e.g., Clawpack or OpenFOAM)? The paper's statement of need should explain the capabilities and why you might opt for this package versus others.

Finally, I think drawing attention to the text input format is counter-productive. One of the advantages of Julia is programmatically creating model configurations and differentiating output with respect to arbitrary inputs. The text format seems to steer away from such things.

@vavrines
Copy link

vavrines commented Jan 18, 2021

I guess the editors may be discussing this article. To avoid possible misunderstandings, I'll write a brief reply here and sorry for any interruptions.

I agree Clawpack is a much more matured software. The main differences are listed here:

  • Clawpack is coded in Fortran and Python, while Kinetic.jl is using Julia and avoids two-language problem
  • It pays considerable attention to the Boltzmann equation, which is missing in Clawpack or OpenFOAM
  • It is closely coupled with scientific machine learning

I believe the text input makes sense. During the runtime, the texts are going to generate variables directly, which relies on the meta-programming property of Julia. Different types of text input (txt, toml, cfg, etc.) are supported, and it provides a convenient way to modify settings without changing source codes. It is worth mentioning that there is an equivalent way to do configuration in Julia scripts without text input. I omitted it in the paper to avoid redundancy.

The documentation isn't perfect at present and is being organized continuously.

Best, Tianbai

@kthyng
Copy link

kthyng commented Jan 19, 2021

@vavrines Can I verify that this submission covers Kinetic.jl and KitBase, KitML and KitFort? Also, have these other packages been published on or are in the process of being published on?

We seem to be narrowing in on what exactly is in this submission and that the packages together are in scope. However, there is still concern about the maturity of the project at this point. The paper is incomplete with no statement of need and a reference to a figure that is not present (?) along with other errors. Also, given that as part of the JOSS review, two reviewers will go through the package(s) in detail as well as all of the documentation, these items should be as perfect as possible now as opposed to some point in the future. Tests and other requirements should also be fully functional before the review starts.

Would you like to pause this submission to improve the documentation and paper before proceeding? This would mean you think you could improve it enough in, say, a few weeks, to be able to proceed with the review. Alternatively, you could withdraw and resubmit at a later time when you have had a chance to make these improvements.

@vavrines
Copy link

vavrines commented Jan 19, 2021

@vavrines Can I verify that this submission covers Kinetic.jl and KitBase, KitML and KitFort? Also, have these other packages been published on or are in the process of being published on?

We seem to be narrowing in on what exactly is in this submission and that the packages together are in scope. However, there is still concern about the maturity of the project at this point. The paper is incomplete with no statement of need and a reference to a figure that is not present (?) along with other errors. Also, given that as part of the JOSS review, two reviewers will go through the package(s) in detail as well as all of the documentation, these items should be as perfect as possible now as opposed to some point in the future. Tests and other requirements should also be fully functional before the review starts.

Would you like to pause this submission to improve the documentation and paper before proceeding? This would mean you think you could improve it enough in, say, a few weeks, to be able to proceed with the review. Alternatively, you could withdraw and resubmit at a later time when you have had a chance to make these improvements.

Dear @kthyng
Thank you very much for the reply. I can confirm this submission covers Kinetic.jl, KitBase.jl, KitML.jl and KitFort.jl. All these packages have never been published on or are in process of being published on any other journal.

And thanks for the comments on the paper. I don't agree with part of the arguments. The statement of need is obviously provided in the summary of paper. The figure is tightly connected to the text above, and I can add a caption of it if necessary (not sure about this point in JOSS). The tests and other requiements work well.

I think the current maturity of the documentation and of the whole package meets the readership of JOSS. However, if it comes to an official decision from editors that the paper submission has to be paused, I would respect it and apply myself to the improvement.

Best,
Tianbai

@kthyng
Copy link

kthyng commented Jan 19, 2021

The statement of need is obviously provided in the summary of paper.

We require a section entitled "Statement of Need." @jedbrown gave suggestions for additions above.

The figure is tightly connected to the text above, and I can add a caption of it if necessary (not sure about this point in JOSS).

The paper seems to imply the presence of a figure but I do not see one. Am I missing something? I see it now in the github representation of the paper but not in the whedon compiled version. Yes you can and should include a caption. The information you need to implement these items should be here.

The tests and other requiements work well.

At a minimum, you can see @jedbrown's comments about the state of the documentation, but this came up in the editorial discussion outside of this issue as well.

The low coverage of tests were brought up as well. Can you elaborate on what is tested for at present?

The items that I am bringing up now are from the editorial board discussion and are also the items that reviewers would need to bring up immediately upon starting their reviews so we try to catch things like this before the review process starts when possible to reduce the time reviewers need to spend. I will pause this paper for now as you work through the items.

@kthyng kthyng added paused and removed query-scope Submissions of uncertain scope for JOSS labels Jan 19, 2021
@kthyng
Copy link

kthyng commented Jan 19, 2021

Ah, I forgot to include above that the paper needs proof reading too. There are a number of grammatical errors at present.

@vavrines
Copy link

@kthyng
Thanks again for the kind explanation.

I figured out that JOSS seems not accepting html syntax so that all the figures in (https://github.com/vavrines/Kinetic.jl/blob/master/paper/paper.md) are missing. Is there a recommended way to adjust the attributes of a picture in JOSS?

I'll work on improving the paper. When it's ready, how should I contact the editor and restart the review process?

Best, Tianbai

@kthyng
Copy link

kthyng commented Jan 19, 2021

@vavrines can you use the example given in the docs that is like this: ![Caption for example figure.](figure.png){ width=20% }

In addition to the paper, please work on the docs and tests.

When you are ready, you can ping @openjournals/joss-eics to start your submission back up.

@vavrines
Copy link

vavrines commented Feb 4, 2021

With the revise of the paper, docs and tests, I would like to reboot the review process.

The major changes include:

  • The paper is rewritten. A statement of need section is added. The figure format is adjusted. The proof reading is made.
  • The tests are expanded and the coverage increases.
  • The documentation is reorganized.

With these changes, the software version is updated to v0.7.1.

@openjournals/joss-eics
@arfon
@kthyng
@Kevin-Mattheus-Moerman
Could you please help continue the review?

@ahwillia
Copy link

ahwillia commented May 4, 2021

I can't, too many deadlines over the next 6 weeks, then a planned vacation.

@vavrines
Copy link

vavrines commented May 6, 2021

@diehlpk
Thanks very much for the continuous support.
If you want I could also recommend some potential reviewers who are working on similar topics on github.

@jarvist
Copy link

jarvist commented May 6, 2021

Hey @Adrianzo @ahwillia @vchuravy @dhhagan @jarvist would you be interested to review this paper?

I'm on parental leave until May 17th, but can then review this.
We have some partial reviews already? The history ^^^^ was a bit long to skim through!

@diehlpk
Copy link
Member

diehlpk commented May 6, 2021

@diehlpk
Thanks very much for the continuous support.
If you want I could also recommend some potential reviewers who are working on similar topics on github.

Yes, please go ahead.

@diehlpk
Copy link
Member

diehlpk commented May 6, 2021

Hey @Adrianzo @ahwillia @vchuravy @dhhagan @jarvist would you be interested to review this paper?

I'm on parental leave until May 17th, but can then review this.
We have some partial reviews already? The history ^^^^ was a bit long to skim through!

Cool, I will assign you as the second reviewer. I have one reviewer already.

@diehlpk
Copy link
Member

diehlpk commented May 6, 2021

@whedon add @jarvist as reviewer

@whedon
Copy link
Author

whedon commented May 6, 2021

OK, @jarvist is now a reviewer

@vavrines
Copy link

vavrines commented May 6, 2021

Hi @fverdugo @sloede
Would you be interested and have time review this paper?

@dhhagan
Copy link

dhhagan commented May 7, 2021

@diehlpk I'm a bit busy with editing right now to take on any reviews, unfortunately. Sorry!

@diehlpk
Copy link
Member

diehlpk commented May 11, 2021

@whedon add @jarvist as reviewer

@whedon
Copy link
Author

whedon commented May 11, 2021

OK, @jarvist is now a reviewer

@fverdugo
Copy link

Hi! thanks for the review invitation @vavrines. I am currently reviewer in another JOSS paper and have no extra time to be involved in a second one unfortunately.

@sloede
Copy link

sloede commented May 12, 2021

I have never reviewed at JOSS, how does it compare to a regular paper review in terms of effort? Also, what is the expected time frime? I am intrigued, but quite busy this and next week...

@vavrines
Copy link

@sloede Thanks for the interest : )
It's depends on a number of factors and hard to make a fair comparison. From my point of view on the checklist given by JOSS, it's easier to review here.
cc @diehlpk I remember the expected time frime you mentioned is 2 months. Given the long delay of the review process, I'm not sure if it stays the same in this round.

@sloede
Copy link

sloede commented May 17, 2021

@vavrines I am sorry, but I have to decline. I wouldn't be able to do it within an appropriate time frame :-/

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

No branches or pull requests