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

Match with slightly unequal timestep times #94

Merged
merged 6 commits into from
Feb 21, 2019
Merged

Match with slightly unequal timestep times #94

merged 6 commits into from
Feb 21, 2019

Conversation

Martin-Rey
Copy link

PR addressing issue #93

  1. The fraction defining two timesteps as having the same time has been moved to the config file.

  2. The missing factor in the "across" keyword to make the test relative rather than absolute has been added.

Feel free to comment
Martin

…st for across directions that should have been relative
@apontzen
Copy link
Member

Looks good, but for some reason the tests are failing...

@Martin-Rey
Copy link
Author

Martin-Rey commented Feb 19, 2019

Yes the reason of test failing is the same as for PR #91 and maybe issue #92, which are all related to (pynbody/pynbody#503). It has to do with compatibility problems between numpy, cython versions when installing pynbody. I gave a go at isolating it in PR #91 but hadn't had the time to follow it up.

@apontzen
Copy link
Member

Oh dear me... I wonder whether @SophiaNasr has any comment, since she managed to fix this bizarre numpy thing in her own installation?

@SophiaNasr
Copy link

SophiaNasr commented Feb 20, 2019

@Martin-Rey have you tried to pip upgrade cython, followed by numpy, and finally, pynbody? This fixed a problem I was having earlier. You can find the versions I updated to in my thread (which looks like it has been cross-referenced by you, so you may have already seen it) pynbody/pynbody#503

@Martin-Rey
Copy link
Author

Thanks @SophiaNasr and @apontzen!

I have done some mix and match of numpy and cython versions and come to the conclusion that it is a pretty inexplicable bug :)

  1. This is not a Tangos issue but rather a pynbody build issue. The failing tangos tests only follow the two pynbody failing tests.

  2. I can get ride of this issue and make the test pass (last build in this pull request) by upgrading manually to numpy 1.16 with cython 0.27 and 0.29. However, this numpy version is not yet available in conda (max 1.15), which is why conda --update does not solve the problem but pip --upgrade does.

  3. We have versions of Tangos and pynbody with cython 0.29 and both numpy 1.10, 1.13 and 1.15 that successfully worked a few month ago (AHF halo ID fix #88) and a few days ago (fix to ahf.py in function _can_load() to take ahf_basename into account when passed to AHFCatalogue pynbody#504).

The only difference I can think off is that both in the Tangos automated build and in @SophiaNasr use case, pynbody was installed via pip, rather than clone+python setup.py.

@apontzen
Copy link
Member

The thing that I don’t understand is that downgrading cython doesn’t seem to help either, which contributes to making this truly baffling.

@apontzen
Copy link
Member

Thanks for your assistance @SophiaNasr! @Martin-Rey if you merge the master branch into this one, travis should now pass...

@apontzen apontzen merged commit 0d9be11 into pynbody:master Feb 21, 2019
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.

3 participants