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

[JOSS Review] Comments on User Guide 3 notebook #45

Open
16 tasks done
CFGrote opened this issue Sep 27, 2022 · 2 comments
Open
16 tasks done

[JOSS Review] Comments on User Guide 3 notebook #45

CFGrote opened this issue Sep 27, 2022 · 2 comments

Comments

@CFGrote
Copy link

CFGrote commented Sep 27, 2022

Re openjournals/joss-reviews#4607

This is a very nice notebook as it clearly demonstrates PyFPT's limitations and points to ways how to overcome these. I found parts of it a bit hard to understand and a number of typos, as listed below.

I don't make a pull request as merging notebooks can be nasty.

  • "the code has flagged that using the 𝑝-values that the lognormal assumption maybe incorrect." -> the code has flagged the distribution as possibly not being lognormal according to the $p$ values.
  • "as we already have the data saved to" -> as we have already saved the data to
  • "re-analysis it" -> re-analyse it
  • "But one can find that the exponential tail shows to the next-to-leading order pole using the advanced root-finding and analysis techniques of" -> I find this sentence hard to understand. Is the "to" in "... shows to the ..." correct?
  • Can you provide a link to the mathematica notebook where this point is analysed? Some readers may be interested. Also, please give the version and a citation/reference link for mathematica.
  • "the scatter of data points is much greater than when bias_amp=0.8" -> the scatter of data points is much greater than with bias_amp=0.8
  • "Second, the 2D histogram shows the spread of N to larger values is much worse than bias=0.8" "Second, the 2D histogram shows the spread of N to larger values is much worse than with bias=0.8"
  • Mathmatica -> Mathematica (see also comment above)
  • "This is case suffers the" "This is case suffers from the "
  • Third, the variance of the weights 𝑤 within each bin increases with T also increases at an even greater rate than the finite 𝜙UV case. -> Third, the variance of the weights 𝑤 within each bin increases with N at an even greater rate than the finite 𝜙UV case.
  • editting -> editing the
  • is used a wrapper -> is used as a wrapper
  • But if one is interested in analysissing the tail of a particular inflation model, a clone of PyFPT can be made and edited it to contain drift and diffusion as ... -> But if one is interested in analysing he tail of a particular inflation model, clone of PyFPT corresponding Cython functions can be added to fpt.numerics.importance_sampling_cython.pyx, in a cloned PyFPT repository (or a fork of the latter).
  • This change will to at least a 2x speed up -> This can speed up calculations by a factor 2 or more.
  • such that region of interest is investigated. -> such that the region of interest is investigated.
  • l technocal -> technical
@CFGrote CFGrote changed the title Comments on User Guide 3 notebook [JOSS Review] Comments on User Guide 3 notebook Sep 27, 2022
@Jacks0nJ
Copy link
Owner

Hi @CFGrote,
Thanks for spotting the typos. I'll update the guide soon to implement your recommendations :).

Will let you know when this is done.

@Jacks0nJ
Copy link
Owner

Jacks0nJ commented Sep 30, 2022

Hi @CFGrote,
I've fixed almost all of the typos you've spotted. But there were a few where I've done things slightly different to how you recommended.

  • Where I referred to using Mathematica, I've simply cited the paper where this is method is detailed instead. For people interested in using PyFPT this particular case is only of interesting for benchmarking the method, and this was done in the paper.

  • For your correction for "editing", the only time I can see I use this word is "manually editing PyFPT", which I don't think should include a "the". But of course let me know if you disagree or I missed the sentence you were referring to.

Do let me know if you spot anything else! And cheers again!

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

No branches or pull requests

2 participants