[yt-dev] Tipsy _is_valid will deadlock loading FLASH datasets

Fri Apr 11 16:45:27 PDT 2014

No worries Matt, I think my code being a bit obtuse is to blame as well.
 It wasn't exactly obvious how to do that extension.  I only just now
realized as well that only the positions in pkdGrav outputs can be double,
so that makes things easier for autodetection.

On Fri, Apr 11, 2014 at 7:40 PM, Matthew Turk <matthewturk at gmail.com> wrote:

>
> On Apr 11, 2014 7:36 PM, "B.W. Keller" <kellerbw at mcmaster.ca> wrote:
> >
> > Alright everyone, I've fixed the problem.  It looks like when we added
> support for pkdGrav's double precision positions, it was implemented in a
> rather slow way, the whole file needed to be loaded and processed (more
> than once).  I've implemented all the filetype checking in a more efficient
> way.  It now only reads ~30 bytes to check if a file is valid.  Here's the
> PR:
> >
> >
> https://bitbucket.org/yt_analysis/yt/pull-request/811/quickly-validate-tipsy-files/diff
> >
> > Ben
>
> Well, my face is red. Thanks to Ben for covering for me, but sounds like
> this was my fault! Sorry everyone.
>
> Relatedly, answer tests are just about back online.
>
> Matt
>
> >
> >
> > On Fri, Apr 11, 2014 at 2:19 PM, Nathan Goldbaum <nathan12343 at gmail.com>
> wrote:
> >>
> >> I have implemented the suggested patch in this commit:
> https://bitbucket.org/yt_analysis/yt/commits/cc6b873a4471e049b6fef33146e92615c72f1243
> >>
> >>
> >> On Fri, Apr 11, 2014 at 10:58 AM, B.W. Keller <kellerbw at mcmaster.ca>
> wrote:
> >>>
> >>> Yep, that's the quickest fix.  I'll send a fix up later today.  Sorry
> about this folks.
> >>>
> >>>
> >>> On Fri, Apr 11, 2014 at 1:18 PM, Sam Skillman <samskillman at gmail.com>
> wrote:
> >>>>
> >>>> +1 to John's implementation. In the interim, tipsy can then still be
> loaded explicitly.
> >>>>
> >>>>
> >>>> On Fri, Apr 11, 2014 at 10:13 AM, John ZuHone <jzuhone at gmail.com>
> wrote:
> >>>>>
> >>>>> Sorry, being a bit more descriptive... by that I mean:
> >>>>>
> >>>>> @classmethod
> >>>>> def _is_valid(self, *args, **kwargs):
> >>>>> return False
> >>>>> ...
> >>>>>
> >>>>> Which is a bit hacky, but the least invasive.
> >>>>>
> >>>>> On Apr 11, 2014, at 1:07 PM, John ZuHone <jzuhone at gmail.com> wrote:
> >>>>>
> >>>>>> +1, except it seems like the easiest thing to do is to just make
> sure that Tipsy's _is_valid always returns False for now.
> >>>>>>
> >>>>>> On Apr 11, 2014, at 12:41 PM, Nathan Goldbaum <
> nathan12343 at gmail.com> wrote:
> >>>>>>
> >>>>>>> Hi all,
> >>>>>>>
> >>>>>>> While Ben works on his fix, would anyone object to temporarily
> reverting the merged pull request that triggered this behavior?
> >>>>>>>
> >>>>>>> I worry about FLASH users who do not read yt-dev.  I guess it *is*
> the bleeding edge, experimental version so bugs should be expected but
> still, reverting seems like an easy temporary fix that takes some pressure
> off Ben to quickly develop a true fix to the underlying issue.
> >>>>>>>
> >>>>>>> Nathan
> >>>>>>>
> >>>>>>> On Thursday, April 10, 2014, B.W. Keller <kellerbw at mcmaster.ca>
> wrote:
> >>>>>>>>
> >>>>>>>> Good idea Mike!  I'll do that too.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Thu, Apr 10, 2014 at 7:39 PM, Mike Warren <mswarren at gmail.com>
> wrote:
> >>>>>>>>>
> >>>>>>>>> You could detect that it is not a tipsy file quickly, by using
> the
> >>>>>>>>> constraint in the header that nbodies=nsph+ndark+nstar and ndim
> is
> >>>>>>>>> presumably 1,2 or 3.
> >>>>>>>>>
> >>>>>>>>> struct tipsy_dump {
> >>>>>>>>>     double time;
> >>>>>>>>>     int nbodies;
> >>>>>>>>>     int ndim;
> >>>>>>>>>     int nsph;
> >>>>>>>>>     int ndark;
> >>>>>>>>>     int nstar;
> >>>>>>>>> };
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> On Thu, Apr 10, 2014 at 5:32 PM, B.W. Keller <
> kellerbw at mcmaster.ca> wrote:
> >>>>>>>>> > Oh my.  Sorry that I have introduced this, unfortunately there
> is no way to
> >>>>>>>>> > detect tipsy files other than actually reading the entire file
> from disk.
> >>>>>>>>> > Perhaps the way to fix this would be to drop the priority of
> tipsy datasets
> >>>>>>>>> > to the bottom, so that a valid FLASH dataset will be detected
> prior to the
> >>>>>>>>> > Tipsy check?
> >>>>>>>>> >
> >>>>>>>>> >
> >>>>>>>>> > On Thu, Apr 10, 2014 at 7:15 PM, Nathan Goldbaum <
> nathan12343 at gmail.com>
> >>>>>>>>> > wrote:
> >>>>>>>>> >>
> >>>>>>>>> >> Hi all,
> >>>>>>>>> >>
> >>>>>>>>> >> We just had a bug report from Aaron Smith at UT Austin.  The
> symptom is
> >>>>>>>>> >> that the "load" comman was taking 30 seconds to complete on
> his FLASH
> >>>>>>>>> >> dataset, which should never happen for FLASH.
> >>>>>>>>> >>
> >>>>>>>>> >> After asking him to profile the code, he produced the
> following profile:
> >>>>>>>>> >>
> >>>>>>>>> >> http://ngoldbaum.net/yt-load/
> >>>>>>>>> >>
> >>>>>>>>> >> It seems that the recent changes to the Tipsy frontend which
> allow it to
> >>>>>>>>> >> autodetect binary outputs have made it so in some cases
> non-tipsy data is
> >>>>>>>>> >> loaded off disk.
> >>>>>>>>> >>
> >>>>>>>>> >> I'm not sure about the best way to handle this, which is why
> I'm writing
> >>>>>>>>> >> to the list rather than issuing a PR.
> >>>>>>>>> >>
> >>>>>>>>> >> -Nathan
> >>>>>>>>> >>
> >>>>>>>>> >
> >>>>>>>>> >
> >>>>>>>>> > _______________________________________________
> >>>>>>>>> > yt-dev mailing list
> >>>>>>>>> > yt-dev at lists.spacepope.org
> >>>>>>>>> > http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >>>>>>>>> >
> >>>>>>>>> _______________________________________________
> >>>>>>>>> yt-dev mailing list
> >>>>>>>>> yt-dev at lists.spacepope.org
> >>>>>>>>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >>>>>>>>
> >>>>>>>>
> >>>>>>> _______________________________________________
> >>>>>>> yt-dev mailing list
> >>>>>>> yt-dev at lists.spacepope.org
> >>>>>>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>> _______________________________________________
> >>>>> yt-dev mailing list
> >>>>> yt-dev at lists.spacepope.org
> >>>>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >>>>>
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> yt-dev mailing list
> >>>> yt-dev at lists.spacepope.org
> >>>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >>>>
> >>>
> >>>
> >>> _______________________________________________
> >>> yt-dev mailing list
> >>> yt-dev at lists.spacepope.org
> >>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >>>
> >>
> >>
> >> _______________________________________________
> >> yt-dev mailing list
> >> yt-dev at lists.spacepope.org
> >> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >>
> >
> >
> > _______________________________________________
> > yt-dev mailing list
> > yt-dev at lists.spacepope.org
> > http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
> >
>
> _______________________________________________
> yt-dev mailing list
> yt-dev at lists.spacepope.org
> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.spacepope.org/pipermail/yt-dev-spacepope.org/attachments/20140411/77bd2971/attachment.htm>