#42 Merged
  1. Matt Turk

This was described in an email to enzo-dev:


UPDATE 1: Add docs, as per Sam's suggestion. Thanks, @Sam Skillman !

UPDATE 2: Adding the file I forgot, and the docs, which got pushed to a different repo!

UPDATE 3: This merges with the recent changes. I've tested that it compiles and goes.

Comments (8)

  1. Sam Skillman

    Hi Matt,

    This looks like a really great start at addressing these issues. I'll go through this as I can in the next few days and build up some more in-depth comments. Just one quick thing I wanted to ping you on is the idea of having these field_objects as a hg subrepo in enzo. I know we haven't used them in the past but since you have this full external library that you've written up with testing/etc, maybe it would be worth including it wholesale through this mechanism? Up to you, but in my mind it might be a good use of that capability, especially since field_objects is really enzo-agnostic at the moment.

    Really nice looking!

    1. Matt Turk author

      Thanks, Sam!

      I'm neutral on subrepos. I actually think in principle it would be very nice to have development of field descriptors occur in an external repository, and that's what we started out doing (as you saw) and even continued doing so into the integration-into-Enzo portion. What came up was that Enzo makes a couple demands on it for integration (which I hadn't completely appreciated). In order, these were:

      • Redefinition of min/max meant this had to be included first, or else map broke. (This might be the most frustrating and easiest one to fix.)
      • "float is double" (Switching to an EnzoFloat instead of float or double might fix the issue of the pointers, but we'll need all the Enzo -D things replicated.)
      • FieldRegistry.C needs the field numbers. (This could simply live always in Enzo.)

      I think these are all possible to deal with, and actually could be backported relatively easily. So I'd like to do so, and keep field_objects somewhat alive for the time being, as I think it ties in nicely with our (you, me, Kacper, Jeff, etc) initiatives for simulation infrastructure. But I don't know that it's right just yet to set it up as a separate repository, since that also means a separate sequence of code review (and it will need to be reviewed) as well as doubling up the way that it gets updated over time.

      (Related note: I think that we may want to take a hard look at how min/max is done, since that it turned out was one of the highest ratio of frustrating to ease of fix I found.)

  2. Nathan Goldbaum

    Hi Matt,

    Sorry I've been slow to respond to this. This is great - it's a massive simplification and a way to eliminate a lot of logic for finding fields that's duplicated throughout the codebase.

    I'm not sure if this is on your roadmaps - but what do you think about iterators for fields? We iterate over grids so much, it would be nice if we could streamline that process a bit.

    1. Matt Turk author

      Hi Nathan -- this does enable iteration over fields, via the Grid object. Not entirely sure what you're thinking of? Do you mean over field elements? If so, I think that's something of a net negative in this instance, but I could otherwise be convinced.

      You can see iteration over fields inside the Interpolation routines.

        1. Matt Turk author

          I think the net increase in verbosity is not really worth it; @Greg Bryan and I did explore that here, but found it cumbersome and overall not worth the extra verbosity since you then have to de-reference the pointer to the iterator iteself. Note that we do have indexing routines, which could also be made to work in reverse.