Mailman 3 Tipsy _is_valid will deadlock loading FLASH datasets - yt-dev

Tipsy _is_valid will deadlock loading FLASH datasets

Nathan Goldbaum

10 Apr 2014 10 Apr '14

11:15 p.m.

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

Attachments:

attachment.html (text/html — 840 bytes)

Show replies by date

B.W. Keller

10 Apr 10 Apr

11:32 p.m.

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 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

John ZuHone

11:38 p.m.

Unless I am mistaken, the problem is that load() runs through all of the _is_valid's regardless. John ZuHone Laboratory for High-Energy Astrophysics NASA/Goddard Space Flight Center 8800 Greenbelt Rd., Mail Code 662 Greenbelt, MD 20771 (w) 301-286-2531 (m) 781-708-5004 john.zuhone@nasa.gov jzuhone@gmail.com

...

On Apr 10, 2014, at 7:32 PM, "B.W. Keller" 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 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@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

Nathan Goldbaum

11:38 p.m.

Hi Ben, There are some sizable test FLASH datasets here: http://yt-project.org/data/ Can you see if you can reproduce it? I think this error will happen for any datasets where yt loads the actual output file rather than a short parameter file. For FLASH there is only one monolithic hdf5 file. I don't think any FLASH-specific fixes will be sufficient. -Nathan On Thu, Apr 10, 2014 at 4:32 PM, B.W. Keller 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 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

Mike Warren

11:39 p.m.

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 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 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@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

B.W. Keller

11 Apr 11 Apr

1:52 a.m.

Good idea Mike! I'll do that too. On Thu, Apr 10, 2014 at 7:39 PM, Mike Warren 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; };

...
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

On Thu, Apr 10, 2014 at 5:32 PM, B.W. Keller wrote: the

...
Tipsy check?

On Thu, Apr 10, 2014 at 7:15 PM, Nathan Goldbaum 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@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

Nathan Goldbaum

4:41 p.m.

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 wrote:

...

Good idea Mike! I'll do that too.

On Thu, Apr 10, 2014 at 7:39 PM, Mike Warren javascript:_e(%7B%7D,'cvml','mswarren@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; };

...
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

...
Tipsy check?

On Thu, Apr 10, 2014 at 7:15 PM, Nathan Goldbaum javascript:_e(%7B%7D,'cvml','nathan12343@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

On Thu, Apr 10, 2014 at 5:32 PM, B.W. Keller javascript:_e(%7B%7D,'cvml','kellerbw@mcmaster.ca');> wrote: the 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@lists.spacepope.orgjavascript:_e(%7B%7D,'cvml','yt-dev@lists.spacepope.org'); http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.orgjavascript:_e(%7B%7D,'cvml','yt-dev@lists.spacepope.org'); http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

John ZuHone

5:07 p.m.

+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 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 wrote: Good idea Mike! I'll do that too.

On Thu, Apr 10, 2014 at 7:39 PM, Mike Warren 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 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 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@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

John ZuHone

5:13 p.m.

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 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 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 wrote: Good idea Mike! I'll do that too.

On Thu, Apr 10, 2014 at 7:39 PM, Mike Warren 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 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 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@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

Sam Skillman

5:18 p.m.

+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 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 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 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 wrote:

...
Good idea Mike! I'll do that too.

On Thu, Apr 10, 2014 at 7:39 PM, Mike Warren 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; };

...
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

...
Tipsy check?

On Thu, Apr 10, 2014 at 7:15 PM, Nathan Goldbaum < nathan12343@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

On Thu, Apr 10, 2014 at 5:32 PM, B.W. Keller wrote: the 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@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

B.W. Keller

5:58 p.m.

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 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 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 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 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 wrote:

...
Good idea Mike! I'll do that too.

On Thu, Apr 10, 2014 at 7:39 PM, Mike Warren 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; };

...
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@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

On Thu, Apr 10, 2014 at 5:32 PM, B.W. Keller wrote: 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@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

Nathan Goldbaum

6:19 p.m.

I have implemented the suggested patch in this commit: https://bitbucket.org/yt_analysis/yt/commits/cc6b873a4471e049b6fef33146e9261... On Fri, Apr 11, 2014 at 10:58 AM, B.W. Keller 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 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 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 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 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 wrote:

...
Good idea Mike! I'll do that too.

On Thu, Apr 10, 2014 at 7:39 PM, Mike Warren 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; };

...
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@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

On Thu, Apr 10, 2014 at 5:32 PM, B.W. Keller wrote: 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@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

B.W. Keller

11:35 p.m.

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... Ben On Fri, Apr 11, 2014 at 2:19 PM, Nathan Goldbaum wrote:

...

I have implemented the suggested patch in this commit: https://bitbucket.org/yt_analysis/yt/commits/cc6b873a4471e049b6fef33146e9261...

On Fri, Apr 11, 2014 at 10:58 AM, B.W. Keller 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 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 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 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 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 wrote:

...
Good idea Mike! I'll do that too.

On Thu, Apr 10, 2014 at 7:39 PM, Mike Warren 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 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@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@lists.spacepope.org > http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org > _______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

Matthew Turk

11:40 p.m.

On Apr 11, 2014 7:36 PM, "B.W. Keller" 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...

...

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

...

...
I have implemented the suggested patch in this commit:

https://bitbucket.org/yt_analysis/yt/commits/cc6b873a4471e049b6fef33146e9261...

...
On Fri, Apr 11, 2014 at 10:58 AM, B.W. Keller

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

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

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 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

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*

...

...
...
...
...
...
> > Nathan > > On Thursday, April 10, 2014, B.W. Keller wrote: >> >> Good idea Mike! I'll do that too. >> >> >> On Thu, Apr 10, 2014 at 7:39 PM, Mike Warren 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 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

wrote: 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. prior to the

...

...
...
...
...
...
>>> > Tipsy check? >>> > >>> > >>> > On Thu, Apr 10, 2014 at 7:15 PM, Nathan Goldbaum < nathan12343@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@lists.spacepope.org >>> > http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org >>> > >>> _______________________________________________ >>> yt-dev mailing list >>> yt-dev@lists.spacepope.org >>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org >> >> > _______________________________________________ > yt-dev mailing list > yt-dev@lists.spacepope.org > http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

B.W. Keller

11:45 p.m.

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 wrote:

...

On Apr 11, 2014 7:36 PM, "B.W. Keller" 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...

...
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

...
...
I have implemented the suggested patch in this commit:

https://bitbucket.org/yt_analysis/yt/commits/cc6b873a4471e049b6fef33146e9261...

...
On Fri, Apr 11, 2014 at 10:58 AM, B.W. Keller

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

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

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 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@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*

...
...
...
...
...
>> >> Nathan >> >> On Thursday, April 10, 2014, B.W. Keller wrote: >>> >>> Good idea Mike! I'll do that too. >>> >>> >>> On Thu, Apr 10, 2014 at 7:39 PM, Mike Warren wrote: >>>> >>>> You could detect that it is not a tipsy file quickly, by using

...
...
...
...
...
>>>> 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@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

wrote: 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. the prior to the

...
...
...
...
...
>>>> > Tipsy check? >>>> > >>>> > >>>> > On Thu, Apr 10, 2014 at 7:15 PM, Nathan Goldbaum < nathan12343@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@lists.spacepope.org >>>> > http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org >>>> > >>>> _______________________________________________ >>>> yt-dev mailing list >>>> yt-dev@lists.spacepope.org >>>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org >>> >>> >> _______________________________________________ >> yt-dev mailing list >> yt-dev@lists.spacepope.org >> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org > >

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

_______________________________________________ yt-dev mailing list yt-dev@lists.spacepope.org http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org

3666

Age (days ago)

3667

Last active (days ago)

List overview

Download

14 comments

6 participants

participants (6)

B.W. Keller
John ZuHone
Matthew Turk
Mike Warren
Nathan Goldbaum
Sam Skillman

Tipsy _is_valid will deadlock loading FLASH datasets

tags

participants (6)