Code quality fixes

[manodeep]

No description provided.

[pep8speaks]

Hello @manodeep! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file Corrfunc/utils.py:

Line 608:80: E501 line too long (80 > 79 characters)
Line 1012:70: E502 the backslash is redundant between brackets
Line 1013:69: E502 the backslash is redundant between brackets

Comment last updated at 2019-10-09 21:06:10 UTC

[manodeep]

@lgarrison Please feel free to add any other code quality fixes onto this PR.

[manodeep]

Before merging this PR, we need to add the bug fix for #192 into the CHANGES.rst file.

[manodeep]

@lgarrison When you have a chance, will you give the changes a look-over, particularly the duplicate ngb-cell check that I just added through e53bce3.

[lgarrison]

Yes, I'll take a look in more detail soon! But just skimming the diff, it looks like we're doing a brute-force check for cell duplication. Is it not possible to simply construct the neighbor list without the duplicates to begin with?

…

On Sat, Aug 24, 2019 at 6:02 PM Manodeep Sinha ***@***.***> wrote: @lgarrison <https://github.com/lgarrison> When you have a chance, will you give the changes a look-over, particularly the duplicate ngb-cell check that I just added through e53bce3 <e53bce3> . — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#189?email_source=notifications&email_token=ABLA7S63L5CCFH57YY2NATTQGGVXBA5CNFSM4H2VUU42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5CIIPA#issuecomment-524583996>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABLA7S72P7IFWZ3Z3WHL2HDQGGVXBANCNFSM4H2VUU4Q> .

[manodeep]

You could almost certainly make the duplication check better. Because that check takes so little time and is likely to come up under very special circumstances, I opted for the brute-force check. Happy to have a brainstorm to see if we can improve the check.

[manodeep]

@lgarrison Will you please take a look at this PR and let me know what you think? We have accumulated a few updates for v2.3.2

[lgarrison]

Looks good! We'll want to keep an eye on the generate_cell_pairs() time, though. I think it can dominate the gridlink time. For a somewhat extreme case of 1e8 particles in nmesh_[x|y|z] = 272, I measure 11 sec in gridlink, 45 sec in generate_cell_pairs(), and 46 sec pair counting.

[manodeep]

Agreed! We need to either add the threads within that generate_cell_pairs or incorporate the async model to generate and consume the cell-pairs