-
Notifications
You must be signed in to change notification settings - Fork 57
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
Docs #205
Conversation
@lgarrison Currently, we are asking people to cite the Related: we need to come up with an easy way to display the citation info from within the code-base (populated automatically based on the code version) |
Maybe we should ask people to cite the MNRAS paper, since that's referred?
It would be great if people cited the proceedings paper too, but maybe
that's overkill for ordinary users (as opposed to developers specifically
interested in the high-performance aspects).
…On Wed, Dec 18, 2019, 12:18 AM Manodeep Sinha ***@***.***> wrote:
@lgarrison <https://github.com/lgarrison> Currently, we are asking people
to cite the ASCL entry for Corrfunc v2.0.0, and then the proceedings in
addition if they are using >= v2.3.0. Are you okay with continuing that?
Other options include asking people to cite the MNRAS paper for v2.0.0
Related: we need to come up with an easy way to display the citation info
from within the code-base (populated automatically based on the code
version)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#205?email_source=notifications&email_token=ABLA7S5522XTODAXKTB3S63QZGXD7A5CNFSM4J3UXPFKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHE4OVA#issuecomment-566871892>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABLA7SZRBHCIX53TIAGGI6LQZGXD7ANCNFSM4J3UXPFA>
.
|
I am conflicted here - asking people to continue citing the ASCL entry means all the citations are reflected by one entry. It has already been time-consuming to track the citations to the Zenodo DOI and then submitting reference corrections to ADS about switching those to the ASCL one. If we ask people to cite the MNRAS paper, then showing the impact will become a little more involved. For instance, here is the citation history for the ASCL entry: Showing such a plot with two bib-entries will be a little more involved. That said, I do not feel strongly about this - @lgarrison if you have strong feelings that new citations should go to the MNRAS paper, then I am happy to go with that. Regarding the conference proceedings, I like the idea of only asking power users to cite that paper. If I understand correctly, you are suggesting that HPC implementations that borrow/modify/heavily use the Corrfunc kernels would cite that paper. |
I think it doesn't much matter which we ask people to cite, because the
citations are going to end up split no matter what we do! Both ASCL and
MNRAS will show up in ADS searches, so some will pick one and some the
other. I don't feel strongly either, but it's kind of nice that we finally
wrote a real code paper about Corrfunc, so maybe it's worth asking people
to start citing that.
That's basically was I was thinking re: the AVX512. If the AVX512 is
particularly enabling for a user, a citation might be appropriate.
…On Wed, Dec 18, 2019 at 2:43 PM Manodeep Sinha ***@***.***> wrote:
I am conflicted here - asking people to continue citing the ASCL entry
means all the citations are reflected by one entry. It has already been
time-consuming to track the citations to the Zenodo DOI and then submitting
reference corrections to ADS about switching those to the ASCL one. If we
ask people to cite the MNRAS paper, then showing the impact will become a
little more involved. For instance, here is the citation history for the
ASCL entry:
------------------------------
[image: Screen Shot 2019-10-11 at 12 51 30 pm]
<https://user-images.githubusercontent.com/3004941/71117307-01a85000-222a-11ea-9d6b-b8c2388c3e33.png>
------------------------------
Showing such a plot with two bib-entries will be a little more involved.
That said, I do not feel strongly about this - @lgarrison
<https://github.com/lgarrison> if you have strong feelings that new
citations should go to the MNRAS paper, then I am happy to go with that.
Regarding the conference proceedings, I like the idea of only asking power
users to cite that paper. If I understand correctly, you are suggesting
that HPC implementations that borrow/modify/heavily use the Corrfunc
kernels would cite that paper.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#205?email_source=notifications&email_token=ABLA7S4FEHUIT7FWMQTUUT3QZJ4NBA5CNFSM4J3UXPFKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHHIGMQ#issuecomment-567182130>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABLA7S26N3RE3RZPCOJ6ZGDQZJ4NBANCNFSM4J3UXPFA>
.
--
Lehman Garrison <lgarrison@flatironinstitute.org>
Flatiron Research Fellow, Cosmology X Data Science Group
Center for Computational Astrophysics, Flatiron Institute
lgarrison.github.io
|
@lgarrison I have updated the citation recommendation in both the README and the docs. In the latest commit, I took at stab at fixing #195 Will you please take a look at this PR when you get a chance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look great, this is will definitely help with #195! Aside from the one comment about the sed
usage, I think this is ready to merge.
Makes sense, thanks!
…On Mon, Dec 23, 2019, 3:00 PM Manodeep Sinha ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In docs/source/apidoc.sh
<#205 (comment)>:
>
if ! python -c 'import numpydoc'; then easy_install --user numpydoc; fi
if ! python -c 'import sphinx'; then easy_install --user sphinx; fi
-sphinx-apidoc -H "Comprehensive API reference" -M -f -o source/api/ ../Corrfunc/
+outdir=source/api
+sphinx-apidoc -H "Comprehensive API reference" -M -f -o $outdir ../Corrfunc/ ../Corrfunc/tests.py ../Corrfunc/call_correlation_functions.py ../Corrfunc/call_correlation_functions_mocks.py
+
+# Fix the blank sub-modules in the Corrfunc file
+for docfile in $outdir/Corrfunc.rst
+do
+ # Delete three lines following the "submodules"
+ sed -e '/Submodules/{N;N;d;}' $docfile > xx
That works on linux but not on OSX, and the one that works on oSX does not
work on linux. That's why I chose this two-step process that works on both.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#205?email_source=notifications&email_token=ABLA7S3LAKACHV4BLB4TUYTQ2EKFPA5CNFSM4J3UXPFKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCQDNYPQ#discussion_r360989145>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABLA7S6BBFPQOJQ5IZL6I7DQ2EKFPANCNFSM4J3UXPFA>
.
|
@lgarrison Instead of hard-coding the name for the temporary file, the name is now auto-generated. Also updated to the proper handling of filenames within shell scripts. I am going to merge this PR in and make the release for 2.3.2 (my) today - is that fine? |
Yes, all fine! Looks ready. |
Fixes #203 and a niggling issue with the docs.
Thanks @1313e for the help!