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

Fri Apr 11 16:35:56 PDT 2014

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

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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.spacepope.org/pipermail/yt-dev-spacepope.org/attachments/20140411/2f58ca66/attachment.html>