Field Objects pull request

24 views
Skip to first unread message

Matthew Turk

unread,
Mar 27, 2013, 5:42:36 PM3/27/13
to enzo...@googlegroups.com
Hi all,

I've just issued the first pull request for field objects in Enzo:

https://bitbucket.org/enzo/enzo-3.0/pull-request/42/first-draft-of-field-objects

Greg and I spent several days last week designing and refining the
design and then implementing field objects. They were inspired both
by discussions that have occurred here as well as Cello and other
field-like systems. We've implemented these first in an isolated
repository (using test-driven development, with very high test
coverage) and then have spent the day importing these into Enzo.

This pull request is against the enzo-dev branch that Sam recently
created. I see no reason this could not also be done to the
week-of-code branch, as (see below) it has been designed as an overlay
with gradual improvements to functionality and infrastructure, rather
than as a backwards-incompatible change. Ultimately, fields are
probably one of the biggest pieces of technical debt in Enzo, and I am
hopeful this is a step in the right direction as Enzo grows in
complexity.

= What Field Objects Do =

Field objects cover many common problems with fields:

* Transparent arithmetic operations from constants
* Transparent field operations such as multiplication, dividing,
subtracting, summing, min/max (optionally via rectangular region
specification and/or overlap with other field descriptors)
* Cell, Face and Edge centering for the purposes of calculating
overlap ("Rect" regions)
* Overlap calculations between Field Descriptor objects
* Names, mappings to names, and unit names (units are more complex
and are not done)
* Allocation management

Essentially, FieldDescriptor objects act as containers for
pointers-to-pointers, and carry with them position, dimensions, rank,
interpolation method, names, units, and centering information.

I've implemented some of this in Enzo. Some work still needs to be
done, but the basis of the FieldObject was designed to first act as an
overlay on top of Enzo in a minimally invasive way, and then to
gradually replace operations. Greg and I see this completely
supplanting DataLabel creation, overlap calculation, interpolation
between grids, and several other things. Additionally, because it
manages the distinction between field dimensions and cell dimensions,
creation and utilization of face/edge/corner-centered fields is
considerably easier.

= Where They Are =

My initial repository for the isolated development, which has now
slightly diverged from what I have put in Enzo, is here:

https://bitbucket.org/MatthewTurk/field_objects

My work on the Enzo-ification of field objects is in the bookmark
field_objects in my main repository:

http://bitbucket.org/MatthewTurk/enzo-3.0

= What They Do Now =

Currently, Field Objects have been inserted into Enzo in a few places.

* grid::InheritProperties now calls grid::ReconstructFieldMap
* grid::InterpolateFieldValues no longer uses the typedefs.h
less-than/greater-than, and in fact no longer calls the fortran
routines.
* EvolveLevel.C conducts a (conditionally-enabled, via -DFIELD_DEBUG)
verification step to ensure the field mapping is still correct.

They still need to be inserted into these areas for minimal viable product:

* Initializers. CollapseTest does this in a minimally-invasive way by
* Several routines you can find in field_objects/MockGrid.C and
field_objects/MockGrid.h will be added for ease of creation of fields.
* FieldDescriptors currently all have "zero" left edge, which means
that they cannot be used for overlap calculations. This is a trivial
fix, but needs to be done in the porting of routines.
* DataLabels can be eliminated
* IO can be rewritten to use this
* FindField can basically be eliminated.

Instead of using something like:

DensNum = FindField( ... )
density = BaryonField[DensNum]
for (i = 0; i < size; i++) {
density[i] *= 10.0;
}

One can simply do:

this->GetField("Density")->Multiply(10);

= Why PR Now? =

I'm going on vacation on Saturday, and I wanted to begin soliciting
feedback on implementation details. I do not expect this to be
accepted without at least a couple rounds of feedback. I
accidentally pushed the changes to the enzo-3.0 repository at first,
and have subsequently stripped them. (Those of you on enzo-vcs may
have been surprised by the email!)

Everything is tested in the field_objects using the google test
framework. The readme under field_objects/ describes how to run them.
(make clean runtests) The tests also demonstrate most of the
functionality.

Here are some of the test files:

https://bitbucket.org/MatthewTurk/enzo-3.0/src/d49217494c74329f54bcf25ce0d8e2c0228ee2a7/src/enzo/field_objects?at=enzo-dev

Specifically:

https://bitbucket.org/MatthewTurk/enzo-3.0/src/d49217494c74329f54bcf25ce0d8e2c0228ee2a7/src/enzo/field_objects/TestCornerCenteredFields.C?at=enzo-dev
https://bitbucket.org/MatthewTurk/enzo-3.0/src/d49217494c74329f54bcf25ce0d8e2c0228ee2a7/src/enzo/field_objects/TestFieldBases.C?at=enzo-dev
https://bitbucket.org/MatthewTurk/enzo-3.0/src/d49217494c74329f54bcf25ce0d8e2c0228ee2a7/src/enzo/field_objects/TestFieldDescriptors.C?at=enzo-dev
https://bitbucket.org/MatthewTurk/enzo-3.0/src/d49217494c74329f54bcf25ce0d8e2c0228ee2a7/src/enzo/field_objects/TestGrid.C?at=enzo-dev

Here is the interpolation routine. I have left in the old code for
comparison purposes.

https://bitbucket.org/MatthewTurk/enzo-3.0/src/d49217494c74329f54bcf25ce0d8e2c0228ee2a7/src/enzo/Grid_InterpolateFieldValues.C?at=enzo-dev

(line 294 and again 408)

The Field Registry is designed to *replace* the typedefs.h system,
which is bursting at the seams:

https://bitbucket.org/MatthewTurk/enzo-3.0/src/d49217494c74329f54bcf25ce0d8e2c0228ee2a7/src/enzo/field_objects/FieldRegistry.C?at=enzo-dev

After a first round of discussion, I'd like to open up discussion of
subsequent integration into operations like boundary correction, IO,
overlap calculation, and so on. I'd be eager to hear suggestions for
future development as well as suggestions for the current
implementation.

Thanks,

Matt

Matthew Turk

unread,
Apr 30, 2013, 1:23:52 PM4/30/13
to enzo...@googlegroups.com
Hi everyone,

I wanted to request again comments and feedback on this. Greg and I
have begun working to integrate these with the communication,
interpolation, zone-filling and so on routines and before that gets
too far along I wanted to again solicit feedback.

Thanks,

Matt
Message has been deleted

Matthew Turk

unread,
Aug 14, 2013, 9:25:55 AM8/14/13
to enzo...@googlegroups.com
Hi Sam,

Thanks very much for your comments!

On Mon, Aug 12, 2013 at 2:06 PM, Sam Skillman <samsk...@gmail.com> wrote:
> Hi Matt,
>
> A couple months later and I've actually gone through the PR quite carefully
> and I think we should move towards including this immediately. In
> particular, I think this will be very useful for IO and boundary
> communication. Beyond that and the tedious task of replacing bits of code
> throughout the codebase to use this syntax, I think there may be very neat
> way of expressing parallelism (OpenMP for example) in the unary and binary
> functions very quickly. This is really a huge infrastructure upgrade!

Ah, that's an interesting point -- I'd love to see some cool
applications of the unary/binary stuff.

>
> One thing that I think would be very helpful since this is a break in the
> usual enzo-isms is a bit of documentation (in the source code). Specifically
> I'd like like to just have the FieldDescriptor.h functions have a short
> explanation for what each function is used to do. Particularly the
> constructors and what (mathematically) each unary/binary overloaded function
> does. I know it is simple in principle, but I think it is really the only
> request I would have before accepting this first iteration before moving
> into the other applications. I'd be happy to help document some of the
> other pieces through PRs as needed.

Okay, I think this is a good point. Do you have a particular format
in mind for this?

>
> Finally I think I have some thoughts on some templated min/max and other
> operators that may help alleviate the min/max pains. I can PR those into
> enzo or the field_objects repo.

Awesome -- I think maybe once the PR goes in it would be okay to just
directly PR into the repo.

>
> I'm going to leave a note on the PR requesting the documentation but outside
> of that I'm ready to hit the approve button!

Thanks! I am still getting back online but I will make this a priority.

-Matt

>
> Sam
> --
> You received this message because you are subscribed to the Google Groups
> "enzo-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to enzo-dev+u...@googlegroups.com.
> To post to this group, send email to enzo...@googlegroups.com.
> Visit this group at http://groups.google.com/group/enzo-dev.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

Sam Skillman

unread,
Aug 14, 2013, 11:26:25 AM8/14/13
to enzo...@googlegroups.com
No particular format, though I really like the way and level that Dan Reynolds documented the EnzoVector class.  I think we should strive to do the same for all new enzo work.  I pointed out a few places with inline comments on the PR itself, but really just some description on what each of the inputs are expected to be. For example just enough docs to tell how each of the left indices are related in GetOverlapRegion, or just mathematically what each of the unary/binary operators do.  I just know otherwise down the road we are all going to get confused as to how each of these things work.
 
>
> Finally I think I have some thoughts on some templated min/max and other
> operators that may help alleviate the min/max pains.  I can PR those into
> enzo or the field_objects repo.

Awesome -- I think maybe once the PR goes in it would be okay to just
directly PR into the repo.

Sounds great!
Reply all
Reply to author
Forward
0 new messages