On Wed, Apr 27, 2016 at 9:12 PM, Hsi-Yu Schive <hyschive@gmail.com> wrote:
Hi all,

I just finish the GAMER frontend for YT, and below I want to summarize some issues and things that might be confusing in the current skeleton frontend. I appreciate if you could help validate these issues and/or clarify some points in the section "Things to be clarified". Hopefully it will make frontend implementation easier in the future. I'll collect all comments and issue a PR for the revised skeleton. It will be complementary to the recent PR #2130 by Jonah Miller, which provides an extremely helpful example frontend.

Thanks so much for doing this. Future frontend authors are indebted to you. I've replied inline where you had questions. 

I think all of the places you're noticing inconsistencies or where the skeleton frontend does things that aren't necessary anymore, you're probably right, and you should go ahead and make the necessary changes to update it. Matt and others might also want to chime in if I get anything wrong...
 


Things to be clarified:
============================
1. Difference between FieldInfoContainer.known_other_fields and GridIndex._detect_output_field:
   => known_other_fields contains fields that *might* be in an output, and lists their units and aliases. _detect_output_fields finds the fields that are defined in the currently loaded dataset.

This is correct. known_particle_fields is the same, but for particle fields.
 

2. Dataset.unique_identifier:
   => ??


This is supposed to an identifier that uniquely identifies the dataset being read in. Some frontends use a string, others use an integer. Some frontends will check if there is some metadata that can be read in from the output file to use for this, but fall back to the creation time of the file. This code from the Enzo frontend might be illustrative:

        if "MetaDataDatasetUUID" in self.parameters:
            self.unique_identifier = self.parameters["MetaDataDatasetUUID"]
        elif "CurrentTimeIdentifier" in self.parameters:
            self.unique_identifier = self.parameters["CurrentTimeIdentifier"]
        else:
            self.unique_identifier = \
                int(os.stat(self.parameter_filename)[stat.ST_CTIME])
 
3. Set AMRGridPatch.Parent = None for grids without parents, and AMRGridPatch.Children = [] for grids without children

4. Field array returned by BaseIOHandler._read_fluid_selection should be contiguous along the z instead of x direction. Therefore, for C-like array with the dimension [x][y][z] and for Fortran-like array with the the dimension (z,y,x), a matrix transpose is required (e.g., using np_array.transpose() or np_array.swapaxes(0,2))

5. start_index, stop_index, and ActiveDimensions in the AMRGridPatch objects
   => ??
   (1) It looks like stop_index is not used anymore

Just grepping from stop_index, I think I agree. We should probably remove it from the frontends where it's defined.
 
   (2) ActiveDimensions will be set by AMRGridPatch._prepare_grid. So perhaps frontend does not need to set it explicitly anymore 
   (3) It seems that start_index is also calculated automatically 
6. chunk, selector, and _read_chunk_data in io.py.
   => ??
   Also the following comment about caching is confusing:
      def _read_chunk_data(self, chunk, fields):
   # This reads the data from a single chunk, and is only used for caching.

Not sure what's up with this.
 

7. float type:
   (1) GridIndex.float_type is the float type for left and right simulation edges and must be float64 now
   (2) BaseIOHandler._read_fluid_selection should always return float64 now even if the on-disk data are stored in single precision

8. Difference between add_output_field and add_field used in FieldInfoContainer.setup_fluid_fields:

9. Dataset._parse_parameter_file: what does the following comment mean: "Note that these are all assumed to be in code units; domain_left_edge and domain_right_edge will be updated to be in code units at a later time"?
   => Perhaps it means that "domain_left_edge and domain_right_edge will be converted to YTArray automatically at a later time"?

Yes, that is a better way of phrasing what happens. 
 

Missing procedure
============================
1. Add the new frontend in frontends/api.py
2. Set GridIndex.max_level
3. Set Dataset.refine_by


Bugin the current skeleton frontend
============================
1. data_structures.py: self.dataset is not set in GridIndex.__init__ when calling self.index_filename = self.dataset.parameter_filename
2. data_structures.py: replace self.Parent=[] by self.Parent = None
3. fields.py: the field_list argument is missing in FieldInfoContainer.__init__. This issue has been fixed in Jonah's PR.
    def __init__(self, ds, field_list):
       super(SkeletonFieldInfo, self).__init__(ds, field_list)


Looking forward to a pull request with fixes!
 

Sincerely,
Hsi-Yu (Justin)

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