First pass at YTEP-6. Closes #484.

#410 Merged at d7b721d
Repository
ngoldbaum
Branch
yt
Repository
yt_analysis
Branch
yt
Author
  1. Nathan Goldbaum
Reviewers
Description

This PR introduces a new Periodicity attribute which now must be attached to StaticOutputs, redefines the Radius field to properly handle periodic boundary conditions, improves periodic_dist, and adds euclidean_dist to math_utils.py.

I've gone through and added code that checks how to set the Periodicity attribute in some of the code frontends. I know this works for enzo, flash, and the stream frontend but since I don't have datasets handy, I was unable to test this for the other frontends I modified. I'm particularly worried that my changes will break the orion and nyx frontends - @caseywstark would you mind taking a look at my modifications to the nyx frontend? If you have any knowledge of the other chombo and boxlib codes, it would be great if you took a look at those frontends as well. I was unable to figure out how to modify the chombo, gdf, maestro, orion, or tiger frontends. I left the gadget frontend alone since it seems to be vestigial. @atmyers, would you mind lending a hand with the orion frontends, and, if you can, the other boxlib/chombo frontends?

@MatthewTurk originally suggested in the notes for Issue #484 that we should use validators and new field definitions to get the radius field to properly update its self to the appropriate definition given the parameter file's periodicity. I did end up implementing such a system but realized that it needlessly added additional complexity. I added some additional justification for not using validators in the commit message for e2239c3.

There's still a fair ways to go before YTEP-6 is done but I think with this change we're well on our way.

Please let me know if you spot any issues or have any style or functional suggestions.

Update 1: Fixing typos (thanks for the docstring Stephen!), adding gdf support (thanks Kacper!), and checking to make sure we pass all the answer and unit tests. On my laptop, all the tests pass except for a couple which error on not being able to create an OutputLog file. Any idea what's going on there? I'm invoking the answer tests in $YT_HG/yt with 'nosetests --with-answer-testing.' I originally tried 'nosetests --with-answer-testing frontends/enzo --answer-name=gold005' but in that case nose didn't find any tests. I'm doing this using my new laptop, maybe I screwed up setting it up? I've got the answer test files and .yt/config file set up correctly as far as I can tell. Here's a pastebin of the output from nose: http://paste.yt-project.org/show/3085/

Update 2: Updating following Matt's comments. This passes all unit and answer tests on my machine.

Update 3: Updating for the orion frontend, thanks for commenting Andrew!

Comments (12)

  1. Kacper Kowalik

    Hi Nathan,

    for gdf you can define periodicity as:

    self.periodicity = ensure_tuple([bnd == 0 for bnd in self.boundary_conditions[::2]])
    
  2. MattT

    Hi Nathan -- as a note, I intend to review this today, but wow, thank you for your work! As far as the answer tests go, my recollection is that they never actually look for OutputLog unless you're running the old framework. I think we can & should just ditch the old framework. Can you try instead doing a two-step process:

    python2.7 setup.py develop python2.7 setup.py nosetests --with-answer-testing

    You can undo develop by running pip uninstall yt.

    1. Nathan Goldbaum author

      That works perfectly. The second command (including nosetests in the setup.py call) is new to me. I'll update the wiki with this information right now.

  3. MattT

    Nathan, this looks very nice to me. My only comment is that we now need to update the YTEP to include a link to this pull request, update the status to "in-progress" and add a column with "Done" for the table of what needs to be changed. I am happy to do that. Thank you!

  4. MattT

    If this addresses all the comments, I'm +1 on accepting once we hear from @caseywstark and @atmyers. Once the YTEP is updated, can you email the user list?

  5. Casey Stark

    Hey Nathan. I just looked at the Nyx data structures change and that looks great. 0 is the right BC flag for periodic. This flag has to be set in any Nyx input file, so no need for a default.

      1. Andrew Myers

        Hi Matt,

        Very similar, except the string to search for is slightly different:

                elif param.startswith("Prob.lo_bc"):
                    self.periodicity = tuple([i == 0 for i in vals])