[yt-dev] ytdata frontend work

Matthew Turk matthewturk at gmail.com
Thu Oct 29 10:52:16 PDT 2015 

Hi Britton,

I spent some time looking at this this morning and thinking about it,
and I'm now not sure that I have a better solution than what you
presented.  A few things, some of which are me writing down things
known to everyone else ...

 * It fakes the grid as particles, which is fine, except for things
that need an extent.  This is the problem identified above with
cell_volume, etc.
 * Some areas in the code may rely on ["index", "dx"] to mean the
actual dx of the index.  I can't think of any offhand, so I'm not sure
that's a thing.  Same for "cell_volume".  There may be places that we
check, say, the total volume in the domain or something, which wants
the index volume.

This is not meant to suggest we should change things so late in the
game, but I am personally wondering if there is a middle ground we
could find, where the values aren't loaded as particles.  But that's
really a bandaid; the solution is more along the lines of allowing
particles to define their own volume.  This will be necessary for
anything where the particle represents a volume (i.e., tesselation
codes), or where the volume is output by the simulation for reasons
such as having a complex metric that needs to be integrated.

I'm speculating here, but it may be the case that we should be
by-default looking for the fluid dx, and if that doesn't exist,
looking for the index dx.  (Or volume.)  I'm not entirely sure how to
make that fallback happen cleanly, but it is out of scope for the PR.
(On the other hand, I'm worried that if such a solution is devised, we
won't remember to undo the fixes you put into this PR.)

Two possibilities I could see:

 * Allow fallbacks to be specified in field tuples.  i.e.,
all_data[("gas", "index"), "cell_volume"] .  I don't particularly like
this.
 * Make derived quantities and things like "cell_mass" whatnot all
first access the field-type "cell_volume", and if that doesn't work,
use "index".  And, during setup, initialize aliases from each fluid
type back to the index fields.  This *would* probably work, and
actually kind of appeals to me.

In typing this all out, I convinced myself that option 2 is the one to
try, and that it's probably feasible.  It would also open us up to
have the ability to more flexibly define indexing systems, and
eventually could maybe get rid of the index fields altogether.  Maybe.
Not sure about that one.

-Matt

On Tue, Oct 27, 2015 at 10:32 AM, Britton Smith <brittonsmith at gmail.com> wrote:
> I was about to send this one:
> http://paste.yt-project.org/show/6000/
>
> Incidentally, it may be better to do this from changeset, 754e08c17816,
> which precedes my adding of a ytdata-specific cell_volume function.  In that
> case, even this is suitable:
> http://paste.yt-project.org/show/6001/
>
> On Tue, Oct 27, 2015 at 3:05 PM, Matthew Turk <matthewturk at gmail.com> wrote:
>>
>> Hi Britton,
>>
>> Yes -- in fact, I am going to try adding it to your PR.  Is there a
>> notebook or script that demonstrates how to make the setup fail as-is?
>>
>> On Tue, Oct 27, 2015 at 10:04 AM, Britton Smith <brittonsmith at gmail.com>
>> wrote:
>> > Hi Matt,
>> >
>> > If we could do that, it would be the cleanest and most effective
>> > solution.
>> > When you have time, can you lay out how this can be done?
>> >
>> > Thanks,
>> > Britton
>> >
>> > On Tue, Oct 27, 2015 at 2:56 PM, Matthew Turk <matthewturk at gmail.com>
>> > wrote:
>> >>
>> >> Hi Britton,
>> >>
>> >> I've thought about this a bit, and I think this can be addressed.  In
>> >> the ytdata frontend, we should be able to make it recognize the index
>> >> fields; if we can't, we are overdue for having coordinate handlers
>> >> check things out.  There should be logic that prevents derived fields
>> >> overriding on-disk fields, so if there is an on-disk field that
>> >> presents "index" it should work.
>> >>
>> >> On Tue, Oct 27, 2015 at 9:35 AM, Britton Smith <brittonsmith at gmail.com>
>> >> wrote:
>> >> > Hi all,
>> >> >
>> >> > I have spent a while looking at the issue involving the derived
>> >> > quantities
>> >> > and I have come up with a solution for some of them, but not all of
>> >> > them.  I
>> >> > have pushed a couple changes to the PR that add a cell_volume and
>> >> > cell_mass
>> >> > field specific to YTDataContainerDatasets that makes use of their
>> >> > "dx",
>> >> > "dy", and "dz" fields.  As of now, the following derived quantities
>> >> > work:
>> >> > - min location
>> >> > - max location
>> >> > - extrema
>> >> > - bulk velocity
>> >> > - total mass
>> >> > - weighted average quantity
>> >> > - weighted variance
>> >> > - total quantity
>> >> >
>> >> > The ones that don't work are:
>> >> > - spin parameter
>> >> > - center of mass
>> >> > - angular momentum vector
>> >> >
>> >> > The final three all don't work for the same reason, which is that
>> >> > they
>> >> > require access to the "index" position fields.  Unfortunately, these
>> >> > cannot
>> >> > be overridden for reasons that have to do with the _determine_fields
>> >> > function.  I have made a point during this development process to
>> >> > alter
>> >> > as
>> >> > little as possible outside of the ytdata frontend.  However, at this
>> >> > point,
>> >> > there is no more room to maneuver.  The only solutions I can come up
>> >> > with
>> >> > are ugly and would make it very difficult to smoothly swap in a
>> >> > proper
>> >> > AMR
>> >> > hierarchy when we get around to creating one and doing this properly.
>> >> >
>> >> > The last thing I can think of would be to swap in special derived
>> >> > quantity
>> >> > functions for this frontend only that use position fields appropriate
>> >> > for
>> >> > the frontend.  The functions themselves would be simple to implement,
>> >> > but
>> >> > because the derived quantity collection is added onto data container
>> >> > objects
>> >> > and not dataset objects, I have no idea how one would do the swap.
>> >> > For
>> >> > a
>> >> > similar reason, I don't know how to simply not allow these problem
>> >> > cases
>> >> > to
>> >> > be called, or to fail in a clean way for them.
>> >> >
>> >> > If anyone has any ideas, I would love to hear them.
>> >> >
>> >> > Britton
>> >> >
>> >> > On Mon, Oct 26, 2015 at 10:12 PM, John ZuHone <jzuhone at gmail.com>
>> >> > wrote:
>> >> >>
>> >> >> No worries—the main issues now are the tests (which don’t run under
>> >> >> Python
>> >> >> 2) and Python 3.
>> >> >>
>> >> >> Tomorrow I’ll point you at the head that I’ve been making changes on
>> >> >> to
>> >> >> address these.
>> >> >>
>> >> >> On Oct 26, 2015, at 6:09 PM, Britton Smith <brittonsmith at gmail.com>
>> >> >> wrote:
>> >> >>
>> >> >> Hi John,
>> >> >>
>> >> >> At the PR hangout, we discussed the possibility of eventually
>> >> >> creating
>> >> >> a
>> >> >> method to extract subsets of AMR hierarchies that would include only
>> >> >> grid
>> >> >> patches that are at least partially contained within the data
>> >> >> container.
>> >> >> With them, we would also include the mask arrays to be applied to
>> >> >> each
>> >> >> grid
>> >> >> patch such that you would only get the cells within the container.
>> >> >> However,
>> >> >> this was deemed something that should not hold up this PR, but that
>> >> >> could
>> >> >> eventually subbed in to fill in any gaps in functionality created by
>> >> >> treating the grid data like particles.  We decided that patching the
>> >> >> existing machinery with a frontend-specific cell_volume field was
>> >> >> the
>> >> >> best
>> >> >> thing as it would get this on front of users faster, especially
>> >> >> since
>> >> >> it
>> >> >> contains a lot of other things that are fully working.  I may have
>> >> >> missed
>> >> >> some important parts of the discussion, but my memory faded a lot
>> >> >> faster
>> >> >> than I thought it would.
>> >> >>
>> >> >> I probably won't have much time to work on this until Friday, but
>> >> >> I'll
>> >> >> try
>> >> >> to play around with the derived quantities when I can.
>> >> >>
>> >> >> Britton
>> >> >>
>> >> >> On Mon, Oct 26, 2015 at 9:45 PM, John ZuHone <jzuhone at gmail.com>
>> >> >> wrote:
>> >> >>>
>> >> >>> Hi all,
>> >> >>>
>> >> >>> Matt asked someone to send a quick note about the current issues
>> >> >>> regarding the ytdata frontend PR:
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>> https://bitbucket.org/yt_analysis/yt/pull-requests/1788/adding-ytdata-frontend
>> >> >>>
>> >> >>> I’m sending it because I’m working on it currently, but I was not
>> >> >>> able
>> >> >>> to
>> >> >>> attend today’s PR hangout so anyone else who was more to add or to
>> >> >>> correct
>> >> >>> should chime in.
>> >> >>>
>> >> >>> Currently, there are 3 issues, from most important to least
>> >> >>> important:
>> >> >>>
>> >> >>> 1. Tests don’t work
>> >> >>> 2. Doesn’t support Python 3
>> >> >>> 3. Derived quantities don’t work
>> >> >>>
>> >> >>> I’m currently working on 1 and 2, and there have been some ideas
>> >> >>> about
>> >> >>> 3.
>> >> >>> The main issue with the latter is that for non-gridded data
>> >> >>> containers
>> >> >>> (like
>> >> >>> sphere, disk, etc.) created from grid datasets, we treat the new
>> >> >>> datasets as
>> >> >>> “particle data”. For that reason there is an issue with calculating
>> >> >>> the
>> >> >>> cell_volume properly. Without that, we cannot use the total_mass()
>> >> >>> quantity.
>> >> >>> Given that we have the cell deltas in the dataset, we could have a
>> >> >>> special
>> >> >>> derived field for the cell_volume.
>> >> >>>
>> >> >>> There are other issues with some of the other derived quantities
>> >> >>> (e.g.,
>> >> >>> weighted_average_quantity returning the wrong units), but I haven’t
>> >> >>> been
>> >> >>> able to diagnose them yet.
>> >> >>>
>> >> >>> I’m working on some changes that will need to be merged into
>> >> >>> Britton’s
>> >> >>> fork eventually to address at least 1 and 2.
>> >> >>>
>> >> >>> Best,
>> >> >>>
>> >> >>> John
>> >> >>>
>> >> >>> _______________________________________________
>> >> >>> 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
>

More information about the yt-dev mailing list