#336 Merged
Repository
Deleted repository
Branch
yt (9f7c233dfb07)
Repository
yt
Branch
yt
Author
  1. Stephen Skory
Reviewers
Description

Update 6 The rockstar source directory is now handled using a rockstar.cfg file instead of an environment variable.

Update 5 Rockstar is now optional when installing. Default is true.

Update 4 I have added Rockstar to the install script and the activate scripts. I have made a patched tar.gz of Rockstar (you can get it here, Matt: http://stephenskory.com/Rockstar-0.99.tar.gz). I ran the install script (making sure to reference my BB repo temporarily) and it looks like it works.

I've also pulled in the most recent mainline and merged.

Let's hope that this is the last update!

Update 3 I wanted to use the new point finding stuff so I merged in from mainline. No other changes.

Update 2.5 I forgot to mention that with this changeset, a small patch is required to Rockstar so that the type of client that is launched can be specified, and not left to the Rockstar server to assign.

Update 2 Big changes!

  • Fixed the MPI/hierarchy hanging issue with the topcomm by initializing the workgroup pool above ParallelAnalysisInterface.__init__(self).
  • Rockstar appears to work inline with Enzo. To do this, I am using forks. Specifically, the server process and the writer/analysis processes are forked, while the reader clients are not forked and are called by all the Enzo MPI threads.
  • I split up the differing functionality in rockstar.py for inline and outline modes into two classes which I know will make Matt happy :).

Update 1 Made some changes in response to Matt's comments. There is an issue with the hierarchy which I've emailed the list about. This is not ready to be pulled in until that's resolved.

Original text below

This includes various improvements and changes to the Rockstar halo finder in yt:

  • num_readers > 1 now works, and can dramatically speed up IO on a parallel disk system.
  • Grids are now read in by readers in a cyclical way which prevents all the root grids being read in by a single reader. Like above, this helps to speed up IO on a parallel disk system.
  • rockstar output is now stored in a directory named "rockstar_halos". Previously it was '"%s_rockstar" % (first pf in time series)', which was confusing when halos for other data snapshots were also stored in that directory.
  • I have added a "rockstar_halos/pf.txt" file that gives the Rockstar index number for each original data snapshot because this is not an obvious connection.

(pf.txt):

# pfname    index
DD0003/data0003 0
DD0004/data0004 1
DD0005/data0005 2
  • The loaded Rockstar halos, read in with LoadRockstarHalos (similar LoadHalos in implementation) now reads in halo data out of the Rockstar binary files (instead of the text files). The Rockstar halos have all the information that Rockstar supplies stored with them in the supp dict (raw - exactly as they are in the binary file, so "pos" below is actually position in Mpc and velocity in km/s), and also the appropriate values attached to the built-in things like total_mass(), center_of_mass() in simulation units where appropriate (like for center_of_mass() and radius).

()

In [2]: rhalos = LoadRockstarHalos(pf, 'rockstar_halos/out_43.list')
yt : [INFO     ] 2012-11-12 13:17:16,696 Initializing Rockstar List
yt : [INFO     ] 2012-11-12 13:17:16,704 Getting the binary hierarchy
yt : [INFO     ] 2012-11-12 13:17:16,714 Finished with binary hierarchy reading
yt : [INFO     ] 2012-11-12 13:17:17,036 Parsing Rockstar halo list
yt : [INFO     ] 2012-11-12 13:17:17,060 Finished rockstar_halos/out_43.list

In [3]: rhalos[0].supp
Out[3]: 
{'J': array([ -6.15271728e+15,  -1.36593609e+17,  -7.80776865e+16], dtype=float32),
 'bulkvel': array([-132.05046082,   11.53190422,   42.16183472], dtype=float32),
 'child_r': 2.6411054,
 'corevel': array([-132.05046082,   11.53190422,   42.16183472], dtype=float32),
 'desc': 0,
 'energy': -8.106986e+21,
 'flags': 1,
 'id': 166,
 'm': 1.5341227e+15,
 'mgrav': 1.5341227e+15,
 'min_bulkvel_err': 1821.8152,
 'min_pos_err': 0.00049575343,
 'min_vel_err': 1821.8152,
 'n_core': 1958,
 'num_child_particles': 2764,
 'num_p': 2409,
 'p_start': 6540,
 'pos': array([   0.20197368,    0.54656458,    0.11256824, -104.33285522,
         29.02485085,   43.5154953 ], dtype=float32),
 'r': 0.018403014,
 'rs': 0.0026318002,
 'rvmax': 1133.2,
 'spin': 0.035755754,
 'vmax': 1877.125,
 'vrms': 1886.2648}
  • Rockstar halos are capable of retrieving particle data from the binary files. Rockstar only stores particle IDs, and these are used to connect to the particle data available in the full data snapshot that is referenced when the Rockstar halos are loaded. The particle data is only read in upon demand.

()

In [4]: rhalos[0]['particle_position_x']
yt : [INFO     ] 2012-11-12 13:40:43,296 Getting 2409 particles from Rockstar binary file rockstar_halos/halos_43.0.bin.
(2409,)
yt : [INFO     ] 2012-11-12 13:40:43,300 Getting particle_index using ParticleIO
yt : [INFO     ] 2012-11-12 13:40:49,674 Getting particle_position_x using ParticleIO
Out[4]: 
array([ 0.21708461,  0.21823348,  0.21871664, ...,  0.20831766,
        0.21101551,  0.21130322])
  • The machinery that sets up Rockstar for halo finding now calculates the total number of particles in parallel, and can fall back if the "particle_type" field is absent.
  • Generally cleaned up the RockstarHalo stuff to more closely resemble the other halo finder Halos in halo_objects.py.
  • Pulled in (by hand) improvements from Chris Moody's Rockstar changes (in pull request #247), including the "force_res" parameter, which I've set to default to the width of the smallest cell in the last (most time evolved) snapshot in the TimeSeries.

Comments (19)

  1. Stephen Skory author

    Shoot, I was mistaken above. The .supp dictionary stores values in converted units, so position and radius are in simulation units, not physical.

  2. MattT

    Hi Stephen, this will take a bit of time to review, but I'll get to it in the next couple days. Thanks for submitting -- initially it looks like a big improvement!

    1. Stephen Skory author

      Hi Matt, thanks for the useful comments you've made so far. Let me know when you're done looking this over and I'll address them.

  3. MattT

    These changes look good to me, other than my comments below. The improvements are certainly worth including.

    Have you had the opportunity to look at my head in MatthewTurk/yt where I have added callbacks to Rockstar? I believe those are now in the released version, and I'd like to include them when we make a big push on Rockstar support.

    Thanks, Stephen!

    1. Stephen Skory author

      I have looked at the nascent callback mechanism, and it looks reasonable.

      If by the "released version," do you mean in the yt_analysis/yt version? Because, if so, it doesn't look like it has made its way over there yet.

      I don't want to have a big conversation about callbacks and halo objects here. But I think that I'm understanding you better than before, and I think it's a good approach. Have you had a chance to think about my email about halo_objects.py I sent on Friday to yt-dev?

  4. MattT

    No, I mean, the released version of Rockstar. :)

    Your email slipped through during the workshop. I'll reply.

  5. MattT

    To be more clear, adding support for callbacks requires a hook in Rockstar, which I supplied to PB in a patch, which I think made its way into the released rockstar version.

  6. MattT

    Hi Stephen, have you had the opportunity to address all the comments? And, do you think this is ready to go?

    1. Stephen Skory author

      Yup, I think I have addressed all your comments and that's included presently. I'd say that it is good to go unless someone objects to using forks and wants to try using threads, which I have no strong opinion about either way.

      As a follow-up, Peter Behroozi replied to my email and asked for the patch so that this stuff below can work without modifying Rockstar. I sent the changes, but it doesn't look like he's pushed them up to google code yet.

  7. Christopher Moody

    Hi Stephen, this is really great. I really look forward to using the Rockstar halo lists. Sorry I didn't see this sooner -- evidently I've had all my bitbucket notifications silenced.

    I'm going to try and test this PR soon.

    What version of Rockstar are you using? Peter sent me a private version yesterday which has two important differences. First, it has better tracking of halos, and second small changes to the config_vars.h template (which means one must manually remove references to variables like OUTPUT_HMAD.)

    chris

    1. Stephen Skory author

      Not having access to the private version, I'm using the public version (0.99) with this diff applied:

      diff -r Rockstar-0.99/client.c /Users/sskory/yt/src/Rockstar-0.99/client.c
      627,628c627,628
      < void client(void) {
      <   int64_t snap, block, chunk, type=-1, i, n, timestep, id_offset;
      ---
      > void client(int64_t in_type) {
      >   int64_t snap, block, chunk, type, i, n, timestep, id_offset;
      640a641,642
      >   type = in_type;
      >
      diff -r Rockstar-0.99/client.h /Users/sskory/yt/src/Rockstar-0.99/client.h
      18c18
      < void client(void);
      ---
      > void client(int64_t type);
      diff -r Rockstar-0.99/main.c /Users/sskory/yt/src/Rockstar-0.99/main.c
      96c96
      <     if (!s) client();
      ---
      >     if (!s) client(-1);
      
  8. Christopher Moody

    I had to remove references to OUTPUT_HMAD, OUTPUT_PARTICLES, DOUBLE_COUNT_SUBHALO_MASS_RATIO, from rockstar_interface.pyx. Otherwise, I tried this out with a single reader / multiple writers and it works beautifully on my ART data.

    The private version of Rockstar I got from Peter has incorporated your suggested changes. I guess whenever he publishes his working version of the code we'll see it in the public.

    Once this is accepted, I'll remove my current PR, merge, and go on from there.

  9. MattT

    Chris, I think it would be sufficient to instead remove the Rockstar portion of your PR and then accept the remainder. Please don't decline it yet.

    So what are we waiting for on this? Peter's new release?

    1. Christopher Moody

      I don't know Peter's release schedule, but this PR works as it stands for both a public version (with a very small patch) and the current private version. I haven't tested the halo objects though.

      Cool, I'll keep the PR around, and update the Rockstar code.

      1. MattT

        Okay, so we still need a patch, and the new public release isn't out yet -- so I think we should hold off on accepting until either we have integrated Rockstar + the patch into the install script, or the new release is out.

  10. MattT

    Hi Stephen, everything looks ready to go to me (modulo testing) except I'd ask that we make Rockstar a variable in the install script, which we can default to on. (Although perhaps this should be up for a vote on yt-dev.) This way in case there are any issues with how the Makefile for Rockstar runs, we can always turn it off. Then, I'm ready!

  11. Stephen Skory author

    I hate to keep this PR going, but I realized that if Rockstar isn't built, I think it will cause problems with the activate scripts which will set ROCKSTAR_DIR anyway. Can you think of the best way to handle this, Matt?

  12. MattT

    Good catch. That had not occurred to me. I actually think it might be epsilon more work, but easier in the long run, to use basically the same thing as in the hdf5.cfg and png.cfg files -- make it an on-disk thing, which rolls back to an env var, and in the install script don't set the ENV var, but do create rockstar.cfg. Does that make sense?

  13. MattT

    Great work. And I think in the future, we'll be able to point to this pull request as an example of our community working together to do really cool things. Thank you, Stephen!