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

Docs #205

Merged
merged 10 commits into from
Dec 23, 2019
Merged

Docs #205

merged 10 commits into from
Dec 23, 2019

Conversation

manodeep
Copy link
Owner

Fixes #203 and a niggling issue with the docs.

Thanks @1313e for the help!

@manodeep manodeep added this to the v2.3.2 milestone Dec 17, 2019
@manodeep manodeep requested a review from lgarrison December 17, 2019 02:50
@manodeep
Copy link
Owner Author

@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)

@lgarrison
Copy link
Collaborator

lgarrison commented Dec 18, 2019 via email

@manodeep
Copy link
Owner Author

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:


Screen Shot 2019-10-11 at 12 51 30 pm


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.

@lgarrison
Copy link
Collaborator

lgarrison commented Dec 20, 2019 via email

@manodeep
Copy link
Owner Author

@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.

Copy link
Collaborator

@lgarrison lgarrison left a 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.

@lgarrison
Copy link
Collaborator

lgarrison commented Dec 23, 2019 via email

@manodeep
Copy link
Owner Author

@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?

@lgarrison
Copy link
Collaborator

Yes, all fine! Looks ready.

@manodeep manodeep merged commit 9b60cea into master Dec 23, 2019
@manodeep manodeep deleted the docs branch December 23, 2019 23:47
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

Successfully merging this pull request may close these issues.

Include AVX512 in the documentation for weights.
2 participants