Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(675)

Issue 59041: HDF5 HaloFinder preloading

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 11 months ago by stephenskory
Modified:
4 months, 3 weeks ago
Reviewers:
matthewturk
CC:
yt-dev_lists.spacepope.org
Base URL:
http://svn.enzotools.org/yt/trunk/yt/lagos/
Visibility:
Public.

Patch Set 1 #

Total comments: 3

Patch Set 2 : DerivedQuantities attempt #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -12 lines) Patch
M HaloFinding.py View 1 7 chunks +18 lines, -9 lines 0 comments Download
M hop/hop_hop.c View 1 1 chunk +1 line, -1 line 0 comments Download
M hop/hop_regroup.c View 1 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 1
matthewturk
14 years, 11 months ago (2009-05-05 16:32:29 UTC) #1
Looks mostly okay, but I think we need to rethink the initial mass calculation
before it gets committed.

http://codereview.appspot.com/59041/diff/1/2
File HaloFinding.py (right):

http://codereview.appspot.com/59041/diff/1/2#newcode197
Line 197: 
Not sure we need this?

http://codereview.appspot.com/59041/diff/1/2#newcode433
Line 433: padded, LE, RE, self._data_source =
self._partition_hierarchy_3d(padding=self.padding)
We should get rid of this initial step of partitioning the entire hierarchy, and
instead move to using a DerivedQuantity.  I'd say something like

all_data = self.pf.h.all_data()
all_data.quantities["TotalQuantity"]("ParticleMassMsun", lazy_reader=True)

which will automatically parallelize.  This would get rid of most of the
following lines, and we could avoid the mpi_allsum, too.  I'll need to add in
the preloading to the DQ object, however.

http://codereview.appspot.com/59041/diff/1/2#newcode450
Line 450: self._data_source.hierarchy.queue)
Should be fine.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b