<div dir="ltr">Hi Rasmi,<div><br></div><div>My apologies for not responding earlier to this. I have been away on holiday the last two weeks and didn't see this.</div><div><br></div><div>Back when the HaloCatalog was first implemented, the .to_json and .from_json functions for saving and loading unit registries did not exist, or I was not aware of them. To avoid confusion, I simply converted all fields to CGS for saving. You can see this around line 435 (in the ._run function) of halo_catalog.py. This is why code_length and CGS units are the same for HaloCatalogDatasets.</div><div><br></div><div>I'm looking over your PR write now and will leave additional comments there.</div><div><br></div><div>Britton</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 2, 2016 at 8:50 PM, Rasmi Elasmar <span dir="ltr"><<a href="mailto:re2300@columbia.edu" target="_blank">re2300@columbia.edu</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Makes sense! <a href="https://bitbucket.org/yt_analysis/yt/pull-requests/2208/added-halo-catalog-unit_registry-saving/diff" target="_blank">PR submitted</a> -- let me know if that's good. Thanks for your help!</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 2, 2016 at 3:15 PM, Nathan Goldbaum <span dir="ltr"><<a href="mailto:nathan12343@gmail.com" target="_blank">nathan12343@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Thu, Jun 2, 2016 at 2:05 PM, Rasmi Elasmar <span dir="ltr"><<a href="mailto:re2300@columbia.edu" target="_blank">re2300@columbia.edu</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Nathan,<div><br></div><div>Sorry for the excess of questions. Should I implement the unit_registry attr check <a href="https://bitbucket.org/yt_analysis/yt/src/b8a09cd382dd34f386ce3634e7f78df3f5d9401d/yt/data_objects/static_output.py?at=yt&fileviewer=file-view-default#static_output.py-802" target="_blank">here</a>? If so, how can I access the h5 dataset from there? If not, is <a href="https://bitbucket.org/yt_analysis/yt/src/b8a09cd382dd34f386ce3634e7f78df3f5d9401d/yt/frontends/halo_catalog/data_structures.py?at=yt&fileviewer=file-view-default#data_structures.py-32" target="_blank">here</a> better? </div></div></blockquote><div><br></div></span><div>I would do it inside the `HaloCatalogDataset` class.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>I don't know what the order of precedence is, so I don't want the creation of the unit registry to be overwritten by the default one _create_unit_registry call in the Dataset __init__. Is Particle File being passed a Dataset that is already initiated with the default UnitRegistry? </div></div></blockquote><div><br></div></span><div>You could add an implementation of _create_unit_regsitry to `HaloCatalogDataset` that supplements the implementation in the base class. First, call the superclass implementation, then if a registry is available in the output file, overwrite it inside `HaloCatalogDataset`. Something like this (using pseudocode here):</div><div><br></div><div> class HaloCatalogDataset(Dataset)</div><div> </div><div> ... snip stuff that's already in this class ....</div><div><br></div><div> def _create_unit_registry(self):</div><div> super(HaloCatalogDataset, self)._create_unit_registry()</div><div> if registry in output_file:</div><div> # replace the registry with what's in the output file</div><span><font color="#888888"><div><br></div><div>-Nathan</div></font></span><div><div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 2, 2016 at 2:18 PM, Nathan Goldbaum <span dir="ltr"><<a href="mailto:nathan12343@gmail.com" target="_blank">nathan12343@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Thu, Jun 2, 2016 at 1:16 PM, Rasmi Elasmar <span dir="ltr"><<a href="mailto:re2300@columbia.edu" target="_blank">re2300@columbia.edu</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Nathan,<div><br></div><div>That approach sounds good. What do you think about this implementation?</div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><font face="monospace, monospace">import h5py<br>import yt</font> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><font face="monospace, monospace">ds = yt.load('RD0009/RD0009')<br>hc_file = h5py.File('halo_catalogs/catalog/catalog.0.h5')</font> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><font face="monospace, monospace"># Include unit_registry as an attribute<br>hc_file.attrs['unit_registry'] = ds.unit_registry.to_json()</font> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><font face="monospace, monospace"># Or as a string dataset<br>unit_registry_json = ds.unit_registry.to_json()<br>str_type = h5py.special_dtype(vlen=str)<br>unit_registry_h5 = hcfile.create_dataset('unit_registry_json', shape=(1,), dtype=str_type)<br>unit_registry_h5[:] = unit_registry_json</font> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><font face="monospace, monospace">hc_file.close()</font> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><font face="monospace, monospace"># Regenerate unit_registry when loading the HaloCatalog?<br>halos_ds = yt.load('halo_catalogs/catalog/catalog.0.h5')<br>hc_file = h5py.File('halo_catalogs/catalog/catalog.0.h5')<br>unit_registry_json = hc_file.attrs['unit_registry']<br>halos_ds.unit_registry = yt.units.unit_registry.UnitRegistry.from_json(</font><span style="font-family:monospace,monospace">unit_registry_json</span><span style="font-family:monospace,monospace">)</span></blockquote><div><br></div><div><br></div><div>I'm not sure if including the unit_registry as an attribute or string dataset is a better idea -- it seems to be an attribute in <a href="https://bitbucket.org/yt_analysis/yt/src/c117d528f18ea412f8b80e8522a0595444e20ae8/yt/units/yt_array.py?at=yt&fileviewer=file-view-default#yt_array.py-796" target="_blank">other parts of the codebase</a>. </div></div></div></blockquote><div><br></div></span><div>Either should be fine. I've actually been meaning to update the snippet you linked to use the JSON method, since pickles aren't portable across python versions, but we need to be careful to maintain backward compatibility with older files.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div><div>Then, how do we get it to take the place of the halos_ds unit_registry? Should this be done on creation in <a href="https://bitbucket.org/yt_analysis/yt/src/b8a09cd382dd34f386ce3634e7f78df3f5d9401d/yt/frontends/halo_catalog/data_structures.py?at=yt&fileviewer=file-view-default#data_structures.py-40" target="_blank">HaloCatalogDataset</a>? (i.e., check if the h5 file has a unit_registry attr, if so, load that, if not use the current defaults?)</div><div><br></div></div></blockquote><div><br></div></span><div>Sounds good! Looking forward to the pull request that implements this.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div></div><div><br></div><div>Thanks,</div><div><br></div><div>Rasmi</div></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 1, 2016 at 1:01 PM, Nathan Goldbaum <span dir="ltr"><<a href="mailto:nathan12343@gmail.com" target="_blank">nathan12343@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Wed, Jun 1, 2016 at 11:40 AM, Rasmi Elasmar <span dir="ltr"><<a href="mailto:re2300@columbia.edu" target="_blank">re2300@columbia.edu</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi all,<div><br></div><div>Greg and I found a bug involving halo catalog unit handling:</div><div><br></div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">>>> halo.quantities['particle_position_x']<br>0.495074982741 cm<br>>>> halo.quantities['particle_position_x'].in_units('code_length')<br>0.495074982741 code_length<br>>>> halo.quantities['particle_position_x'].in_units('cm')<br>0.495074982741 cm<br>>>> ds.unit_registry['code_length']<br>(9.195880139956267e+25, (length))<br>>>> halos_ds.unit_registry['code_length']<br>(1.0, (length))</blockquote><div><br></div><div>The halos_ds mixes up cm and code_length units when the HaloCatalog object is created from a saved halo catalog. The halo catalog values are saved in code_length, but the HaloCatalog object assumes they are in cm. </div></div><div><br></div><div><a href="https://bitbucket.org/yt_analysis/yt/src/b8a09cd382dd34f386ce3634e7f78df3f5d9401d/yt/analysis_modules/halo_analysis/halo_catalog.py?at=yt&fileviewer=file-view-default#halo_catalog.py-453" target="_blank">Here</a> is where the code_length units are written out after halo-finding is done (this is confirmed with an h5ls).</div><div>Here is where the <a href="https://bitbucket.org/yt_analysis/yt/src/b8a09cd382dd34f386ce3634e7f78df3f5d9401d/yt/frontends/halo_catalog/data_structures.py?at=yt&fileviewer=file-view-default#data_structures.py-78" target="_blank">halos_ds</a> for the HaloCatalog is created. The length unit is set in cm -- the catalog is assumed to be in cgs.</div><div>The <span></span><a href="https://bitbucket.org/yt_analysis/yt/src/b8a09cd382dd34f386ce3634e7f78df3f5d9401d/yt/frontends/halo_catalog/fields.py?at=yt&fileviewer=file-view-default#fields.py-21" target="_blank">HaloCatalog<span></span> fields</a> also assume cgs.</div><div><br></div><div>In theory, the HaloCatalog could just parse the code_length units of the halos_ds, but this isn't necessarily known at the time of creation, so the ideal fix may be to save the halo catalog length units in cm instead of in code_length. Then the assumptions that are made about length being in cm when creating a HaloCatalog object from a halo catalog would be correct. Any thoughts on this approach or other approaches?</div><div><br></div></div></blockquote><div><br></div></span><div>It would probably be better just to save the unit registry into the hdf5 file. You might find the UnitRegistry.to_json() and UnitRegistry.from_json() to be useful here - the json data could be saved in the HDF5 output file as a string dataset.</div><div><br></div><div><a href="https://bitbucket.org/yt_analysis/yt/src/yt/yt/units/unit_registry.py?at=yt&fileviewer=file-view-default#unit_registry.py-120" target="_blank">https://bitbucket.org/yt_analysis/yt/src/yt/yt/units/unit_registry.py?at=yt&fileviewer=file-view-default#unit_registry.py-120</a><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div></div><div><br></div><div>Thanks,</div><div><br></div><div>Rasmi</div><div><br></div></div>
<br>_______________________________________________<br>
yt-dev mailing list<br>
<a href="mailto:yt-dev@lists.spacepope.org" target="_blank">yt-dev@lists.spacepope.org</a><br>
<a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" rel="noreferrer" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
<br></blockquote></div><br></div></div>
<br>_______________________________________________<br>
yt-dev mailing list<br>
<a href="mailto:yt-dev@lists.spacepope.org" target="_blank">yt-dev@lists.spacepope.org</a><br>
<a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" rel="noreferrer" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
<br></blockquote></div><br></div>
</div></div><br>_______________________________________________<br>
yt-dev mailing list<br>
<a href="mailto:yt-dev@lists.spacepope.org" target="_blank">yt-dev@lists.spacepope.org</a><br>
<a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" rel="noreferrer" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
<br></blockquote></div></div></div><br></div></div>
<br>_______________________________________________<br>
yt-dev mailing list<br>
<a href="mailto:yt-dev@lists.spacepope.org" target="_blank">yt-dev@lists.spacepope.org</a><br>
<a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" rel="noreferrer" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
<br></blockquote></div><br></div>
</div></div><br>_______________________________________________<br>
yt-dev mailing list<br>
<a href="mailto:yt-dev@lists.spacepope.org" target="_blank">yt-dev@lists.spacepope.org</a><br>
<a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" rel="noreferrer" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
<br></blockquote></div></div></div><br></div></div>
<br>_______________________________________________<br>
yt-dev mailing list<br>
<a href="mailto:yt-dev@lists.spacepope.org" target="_blank">yt-dev@lists.spacepope.org</a><br>
<a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" rel="noreferrer" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
<br></blockquote></div><br></div>
</div></div><br>_______________________________________________<br>
yt-dev mailing list<br>
<a href="mailto:yt-dev@lists.spacepope.org">yt-dev@lists.spacepope.org</a><br>
<a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" rel="noreferrer" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
<br></blockquote></div><br></div>