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]: fgivenx: a python package for functional posterior plotting #834

Closed
whedon opened this issue Jul 24, 2018 · 16 comments
Closed

Comments

@whedon
Copy link

whedon commented Jul 24, 2018

Submitting author: @williamjameshandley (Will Handley)
Repository: https://github.com/williamjameshandley/fgivenx
Version: v2.1.0
Editor: @arfon
Reviewers: @FaustinCarter, @zhampel

Author instructions

Thanks for submitting your paper to JOSS @williamjameshandley. The JOSS editor (shown at the top of this issue) will work with you on this issue to find a reviewer for your submission before creating the main review issue.

@williamjameshandley if you have any suggestions for potential reviewers then please mention them here in this thread. In addition, this list of people have already agreed to review for JOSS and may be suitable for this submission.

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 Jul 24, 2018

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @arfon it looks like you're currently assigned as the editor for this paper 🎉

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

@whedon commands

@whedon
Copy link
Author

whedon commented Jul 24, 2018

Attempting PDF compilation. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Jul 24, 2018

@arfon
Copy link
Member

arfon commented Jul 24, 2018

👋 @williamjameshandley - could you please take a look at this list of potential reviewers and identify some potential candidates to review your submission? https://bit.ly/joss-reviewers

@williamjameshandley
Copy link

pragyansmita
nilesh-patil
ygrange
mattpitkin
zingale
adam-m-jcbs
ziotom78
wtgee
benjaminrose
Cadair
zhampel
nespinoza
FaustinCarter

@arfon
Copy link
Member

arfon commented Jul 24, 2018

👋 @zhampel @FaustinCarter - would you be willing to review this submission for JOSS?

@FaustinCarter
Copy link

Sure. I can do that. Give me a week or so to look over the code. After skimming the paper quickly I think it could benefit from a rewrite. Some specific comments by paragraph:

1:
-Why does this end in a colon? Are you about to present an example?
-Your tense is off. I think you mean "probability distributions" rather than "a probability distribution" (which would imply a single distribution for all unknown parameters in the universe!)

2:
-I'm not sure what the point of this paragraph is. The age of the universe and the idea that space expands don't seem to be relevant at any further point in the paper.

3:
-Plots like what? Are you suggesting that the idea in the above paragraph could be parameterized into two variables that have a joint probability distribution? What are those variables? Can you include the plot of those variables made from your software?

4:
-scipy, getdist, and corner all have instructions on how to cite them in an academic publication. It would be good to include those citations in addition to links to their webpages. A reference to a MCMC paper would also be good to make it easy for readers to look that up.
-I'll also put in a shameless plug here for the pygtc package (10.21105/joss.00046) which is an alternative to corner and probably belongs in that list, although that's up to you.
-After a more careful reading I think you are implying that corner plots are nice, but there is more to the story (i.e. looking at the actual model described by all the MCMC realizations?) and that's where your software comes in? Might be good to make this more explicit.

5:
-Mixed voice. You switch between third person (one does, one could) and first person plural (we do, we could, our ability). It would be good to unify the voice over the whole paper.
-Is the phrase "plotted thus" supposed to be a reference to Fig. 1? If so, say something like: "in Fig. 1 panel (a) we plot the parameter covariances between m and c for X realizations. In panel (b) we plot the function y=mx+c for X realizations. In panel (c) we plot something called DxL which is never mentioned in the paper and should be. In panel (d) we plot the probability of measuring y for a given x, which is essential a contour version of panel (b)."
-Why is there a red distribution and a blue distribution? Is that two separate sets of fake data that are being fit to the same model?

6:
-A couple references seem broken?

7:
-The first line of this is the first mention you make of what your package actually does in words. I think this should come earlier and there should be a little more description of what fgivenx is designed to do right from the start.

That's enough for now. Let me dig into the code an run it and I'll have more comments. Oh, and is there a link to hosted documentation somewhere (I didn't see one on the github page)? Preferably documentation that includes installation instructions and a description of options (see the docs for corner as a great example)?

@arfon
Copy link
Member

arfon commented Jul 25, 2018

Thanks for agreeing to work on this @FaustinCarter. I'd like to find a couple of reviewers for this submission. Once I've identified a second reviewer, I'll go ahead and set up the main review issue which will provide a checklist for you to work through.

@zhampel
Copy link

zhampel commented Jul 26, 2018

Hey @arfon, yes I'd be willing to review, however, I will realistically be able to take a close look in a week. If that's too long of a delay, then feel free to request another reviewer.

@arfon
Copy link
Member

arfon commented Jul 26, 2018

@whedon add @FaustinCarter as reviewer

@whedon whedon assigned arfon and unassigned arfon Jul 26, 2018
@whedon
Copy link
Author

whedon commented Jul 26, 2018

OK, @FaustinCarter is now a reviewer

@arfon
Copy link
Member

arfon commented Jul 26, 2018

@whedon add @zhampel as reviewer

@whedon
Copy link
Author

whedon commented Jul 26, 2018

OK, @zhampel is now a reviewer

@arfon
Copy link
Member

arfon commented Jul 26, 2018

@whedon start review

@whedon
Copy link
Author

whedon commented Jul 26, 2018

OK, I've started the review over in #849. Feel free to close this issue now!

@arfon
Copy link
Member

arfon commented Jul 26, 2018

@FaustinCarter & @zhampel, many thanks for agreeing to review this submission. Please head over to #849 to carry out your review.

@arfon arfon closed this as completed Jul 26, 2018
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