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]: PyGModels: A Python package for exploring Probabilistic Graphical Models with Graph Theoretical Structures #3015

Closed
whedon opened this issue Feb 5, 2021 · 27 comments

Comments

@whedon
Copy link

whedon commented Feb 5, 2021

Submitting author: @D-K-E (D. Kaan Eraslan)
Repository: https://github.com/D-K-E/graphical-models/
Version: v0.1.0-beta
Editor: @dfm
Reviewers: @eigenfoo, @ankurankan
Managing EiC: Kyle Niemeyer

⚠️ 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 @D-K-E. Currently, there isn't an JOSS editor assigned to your paper.

@D-K-E 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 Feb 5, 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 Feb 5, 2021

Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=2.41 s (138.3 files/s, 17897.2 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
HTML                           213            771           1883          27587
Python                          37           1039           1766           5920
CSS                              3            328             47           1627
JavaScript                      75            105            144           1573
Markdown                         2             96              0            231
YAML                             3              4              3             56
TeX                              1             13              0             22
-------------------------------------------------------------------------------
SUM:                           334           2356           3843          37016
-------------------------------------------------------------------------------


Statistical information for the repository '97e768c100e4576121bfd97b' was
gathered on 2021/02/05.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Qm Auber                        66         15033           4486          100.00

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Qm Auber                  10547           70.2          0.5               10.64

@whedon
Copy link
Author

whedon commented Feb 5, 2021

PDF failed to compile for issue #3015 with the following error:

/app/vendor/ruby-2.6.6/lib/ruby/2.6.0/psych.rb:456:in parse': (518af8b02e8392cac0c6845a/paper/paper.md): could not find expected ':' while scanning a simple key at line 26 column 1 (Psych::SyntaxError) from /app/vendor/ruby-2.6.6/lib/ruby/2.6.0/psych.rb:456:in parse_stream'
from /app/vendor/ruby-2.6.6/lib/ruby/2.6.0/psych.rb:390:in parse' from /app/vendor/ruby-2.6.6/lib/ruby/2.6.0/psych.rb:277:in load'
from /app/vendor/ruby-2.6.6/lib/ruby/2.6.0/psych.rb:578:in block in load_file' from /app/vendor/ruby-2.6.6/lib/ruby/2.6.0/psych.rb:577:in open'
from /app/vendor/ruby-2.6.6/lib/ruby/2.6.0/psych.rb:577:in load_file' from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-92346a0773a4/lib/whedon.rb:127:in load_yaml'
from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-92346a0773a4/lib/whedon.rb:87:in initialize' from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-92346a0773a4/lib/whedon/processor.rb:38:in new'
from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-92346a0773a4/lib/whedon/processor.rb:38:in set_paper' from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-92346a0773a4/bin/whedon:58:in prepare'
from /app/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor/command.rb:27:in run' from /app/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in invoke_command'
from /app/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor.rb:387:in dispatch' from /app/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor/base.rb:466:in start'
from /app/vendor/bundle/ruby/2.6.0/bundler/gems/whedon-92346a0773a4/bin/whedon:131:in <top (required)>' from /app/vendor/bundle/ruby/2.6.0/bin/whedon:23:in load'
from /app/vendor/bundle/ruby/2.6.0/bin/whedon:23:in `

'

@D-K-E
Copy link

D-K-E commented Feb 5, 2021

Here is a list of people that might be interested in reviewing this software:
juliohm
matt-graham
eigenfoo
1313e
shahmoradi
acguidoum

@kyleniemeyer
Copy link

Hi @D-K-E, thanks for your interest in JOSS. We just this pre-review issue to assign the editor and reviewers, and resolve any issues.

First, regarding the compilation error in your paper, I think you need --- after the YAML header, before the # Summary line.

Second, your paper needs a bit of work. For one thing, I do not think that the summary is actually written for a non-specialist audience, because reading it I am not really sure what the purpose/functionality is. Also, the rest of the paper should not be part of the Statement of Need—that is typically around one paragraph. You can have other sections beyond Summary and Statement of Need, to describe the functionality, uses, etc. of the software. You should also connect the software to some theory, via citations to relevant articles.

@D-K-E
Copy link

D-K-E commented Feb 6, 2021

Hi @kyleniemeyer thanks for the input. I'll try to fix the compilation error. It compiled fine under the web service, but I'll add the line.
I'll clarify the points you had pointed out as well.
How should I inform JOSS once the changes are made ?

@kyleniemeyer
Copy link

@D-K-E you can comment in here for all communication with us—but, in this case, you can generate the pdf with @whedon generate pdf

@kyleniemeyer
Copy link

@D-K-E there's no rush on this, just comment here when you have finished making the changes. (Also, for that compilation issue, that was just my guess—it might be something else.)

I'm going to label this "paused", but just ping us when you have made the changes.

@kyleniemeyer
Copy link

Hey @dfm, could you edit this one? It's paused while the author revises the paper.

@dfm
Copy link

dfm commented Feb 6, 2021

I'm happy to edit!

@dfm
Copy link

dfm commented Feb 6, 2021

@whedon assign @dfm as editor

@whedon
Copy link
Author

whedon commented Feb 6, 2021

OK, the editor is @dfm

@D-K-E
Copy link

D-K-E commented Feb 9, 2021

@dfm @kyleniemeyer I have made couple of changes to the paper as per requested. Here is its summary:

  • Paper compiles now.
  • Added References section for citations.
  • Reworded Summary to make it more clear to general public.
  • Statement of Need tries to demonstrate the research related aspect through a use case and citations.
  • Added Applications and Similar Works section which contains a very small review of similar libraries and how PyGModels compares against them. During the comparison, I also cite relevant sources when I am introducing a novelty with respect compared libraries.

A simple question, I cite some sources in the source code but not in the paper should I include them to References section as well ?

@kyleniemeyer
Copy link

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Feb 9, 2021

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

@kyleniemeyer
Copy link

Thanks @D-K-E. The paper is definitely improved, and I'm going to leave it to @dfm now (aside from the following comments 😅).

I do think it makes sense to cite things in the paper that you cite in the source code—this is actually a great way of acknowledging those sources to give them the appropriate academic credit.

The Summary (and rest of the paper) is better, but I would suggest trying to make the introduction even more general. As written, it still does not quite satisfy the requirement (in my view):

A summary describing the high-level functionality and purpose of the software for a diverse, non-specialist audience.

@D-K-E
Copy link

D-K-E commented Feb 24, 2021

Hello again @dfm I have improved the documentation a little more. Would you like me to change something else ? And are we still pausing the review for this project ?

@dfm
Copy link

dfm commented Mar 6, 2021

@D-K-E: Thanks for your patience! The paper is looking better, but I think that the online documentation could still use some work to make it more accessible. Right now the README seems to be the main source of documentation with the doxygen API docs hosted separately. It would be good to connect these (probably by moving most of the functionality discussion to the doxygen pages, but feel free to structure it differently) and add more context because right now it's very hard to navigate and see how the different pieces fit together.

Also, can you explain the relationship between this repo and https://github.com/Viva-Lambda/graphical-models?

@D-K-E
Copy link

D-K-E commented Mar 6, 2021

@dfm Hi ! It is really nice to here from you. The Viva-Lambda repository, is also me, a part of the code is live coded in my twitch channel (twitch.tv/vivalambda) and some of my viewers, mostly graduate students in CS, were interested in seeing the code, so I uploaded it to channel's repository as well. I can remove it, if you feel it's necessary, or add a notice to readme which repo is the original one. I tend to use the channel's repository as a scratch book (test new ideas, functionality, discover new apis, etc), and use my personal repository, D-K-E to publish more stable versions.

For the documentation, I will move the "Currently Supported Models" to doxygen main page, and continue to add doc strings to functions. Does that sound alright ?

@D-K-E
Copy link

D-K-E commented Mar 13, 2021

@dfm I have moved usage examples to doxygen main page as I understood from:

It would be good to connect these (probably by moving most of the functionality discussion to the doxygen pages, but feel free to structure it differently)

I'll see what I can do about adding more context. Would you mind giving an example repository who does this well ? I am not sure how to approach it

@dfm
Copy link

dfm commented Mar 15, 2021

@D-K-E:

  • Thanks for the clarification re: Viva-Lambda; I think it would be good to add a link to that README highlighting that the D-K-E version is the primary repo and clarifying the relationship, but otherwise all sounds good!
  • I think that moving the README content to the doxygen pages really helps and it's getting closer. I'd say that the main major component that is still missing is the Example Usage review criterion. The one example is good, but I think it would be useful to have more extensive examples. The pgmpy docs (for example) are a pretty good example of what we're looking for. Yours certainly does not need to be quite so extensive, but those docs are a good example of the structure, I think.

I'll start looking for reviewers while you chip away at this. Let me know if you have further questions!

@dfm dfm removed the paused label Mar 15, 2021
@dfm
Copy link

dfm commented Mar 15, 2021

@whedon assign @eigenfoo as reviewer

@eigenfoo has agreed to review - thanks George!! Once we have a second reviewer, I'll get the review started in a separate thread.

@whedon whedon assigned dfm and unassigned dfm Mar 15, 2021
@whedon
Copy link
Author

whedon commented Mar 15, 2021

OK, @eigenfoo is now a reviewer

@dfm
Copy link

dfm commented Mar 16, 2021

@whedon add @ankurankan as reviewer

Ankur will be our second reviewer - thanks!!

@D-K-E, @eigenfoo, @ankurankan: I'll start the main review thread now so all conversation will move there. Please don't hesitate to ask if any of you have any questions as the review progresses. Thanks all for participating!

@whedon whedon unassigned dfm and eigenfoo Mar 16, 2021
@whedon
Copy link
Author

whedon commented Mar 16, 2021

OK, @ankurankan is now a reviewer

@dfm
Copy link

dfm commented Mar 16, 2021

@whedon start review

@whedon
Copy link
Author

whedon commented Mar 16, 2021

OK, I've started the review over in #3115.

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

5 participants