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

Fix Memory Error when data is larger than chunk size in paint(). #442

Merged
merged 3 commits into from Nov 15, 2017

Conversation

Projects
None yet
3 participants
@rainwoodman
Member

rainwoodman commented Nov 10, 2017

@rainwoodman rainwoodman requested a review from nickhand Nov 10, 2017

@rainwoodman

This comment has been minimized.

Member

rainwoodman commented Nov 10, 2017

@zdplayground :

You can test this after running:

 pip install -U --no-deps https://github.com/rainwoodman/nbodykit/archive/large-data.zip

In your conda environment of nbodykit; then restart the kernel.

@zdplayground

This comment has been minimized.

zdplayground commented Nov 11, 2017

@rainwoodman:
After installing large-data.zip, the memory error disappears, though the running time of FFTPower for such case seems a little bit long.

# be sure to use the source to compute
position, weight, value = \
self.base.compute(Position[s], Weight[s], Value[s])
self.base.compute(Position[s][sel], Weight[s][sel], Value[s][sel])

This comment has been minimized.

@nickhand

nickhand Nov 11, 2017

Member

I think we might want to apply sel to the arrays after computing to be safer (this was the logic used in 4e41aab).

@nickhand

everything else looks good to me

@rainwoodman

This comment has been minimized.

Member

rainwoodman commented Nov 11, 2017

@zdplayground it's 1 billion particles on a single core; that's why I usually run this from the job queue with ~ 100 cores.

@rainwoodman

This comment has been minimized.

Member

rainwoodman commented Nov 11, 2017

@nickhand let's wait till travis is green before merging.

@nickhand nickhand merged commit aae31fb into bccp:master Nov 15, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.005%) to 95.294%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment