Make CHUNKSIZE into an index property #2658

matthewturk · 2020-06-20T18:04:35Z

Currently, the variable CHUNKSIZE lives at the top level of the particle_geometry_handler.py file. This should instead be an index attribute.

(Note that #2626 is a bit related to this.)

This also adds the ability to set it to 0 to indicate it's unlimited, although this may cause issues with some frontends; that needs a bit more testing.

This -- or something like it -- may be required to get the OWLSSubfindDataset class more easily used.

matthewturk · 2020-06-23T15:21:21Z

@yt-fido test this please

Xarthisius · 2020-06-29T17:34:56Z

@yt-fido test this please

neutrinoceros

Just one minor suggestion: since the attribute is meant to accessed from outside the class, maybe it should either not have a leading underscore, or be accessed through a property. This second option is best if there's a reason to forbid the end user to change this attribute.

matthewturk · 2020-07-19T18:59:38Z

well, it's kind of a "friend" attribute. So I was not completely sure what to do. I suppose it's OK to make it dunder or a property, though, to make sure, since changing it mid-stream can be problematic.

neutrinoceros · 2020-07-19T19:02:02Z

well, it's kind of a "friend" attribute. So I was not completely sure what to do. I suppose it's OK to make it dunder or a property, though, to make sure, since changing it mid-stream can be problematic.

yes I think that's the closer it gets to defining friends in Python 👍

neutrinoceros

awesome ! thanks for the refactor !

matthewturk · 2020-09-16T14:17:47Z

I'm going to fix these conflicts and attempt again.

matthewturk · 2020-09-18T15:02:03Z

/black

neutrinoceros · 2020-09-18T15:02:25Z

Feel free to ping me when this is ready for a final review, I feel like it has already changed enough that I should go over it again at some point :-)

matthewturk · 2020-09-18T15:14:26Z

I think it's ready

neutrinoceros · 2020-09-18T15:38:57Z

Ok I'll go over it then.

This -- or something like it -- is required to get the OWLSSubfindDataset class working again, and to get #2437 in.

This is now a bit outdated I guess since the PR in question was in fact merged before this one. Could you explain why it is so and/or edit the description ?

matthewturk · 2020-09-18T15:40:07Z

I actually can't remember why I thought it was necessary. I still think it's good though.

neutrinoceros

One question before I read everything in detail: wouldn't it be worth having the chunk_size property defined in BaseIOHandler as

@property
def chunk_size(self):
    raise NotImplementedError("something something")

so as to keep uniformity in all subclasses ?

I also don't understand why it need to be defined in ParticleIndex as well, which looks unrelated to BaseIOHandler. Is there a way to define this set property in one base class only and make the other one get the value from there ?

neutrinoceros · 2020-09-18T15:57:54Z

Looks like you're doing just that in some places already, e.g.

ind += self.ds.index.chunksize

So I think a clean pattern would be

# in ParticleIndex
@property
def chunk_size(self):
    raise NotImplementedError("blabla")

# in BaseIOHandler
@property
def chunk_size(self):
    return self.ds.index.chunk_size

and of course have the appropriate definitions in supported ParticleIndex's child classes

neutrinoceros · 2020-09-18T17:29:58Z

Upon further reflection maybe it's even better to make ParticleIndex an abstract class instead of throwing a NotImplementedError

Xarthisius · 2021-01-21T15:33:27Z

@yt-fido test this please

brittonsmith · 2021-01-21T15:55:37Z

@yt-fido test this please

Oops, didn't see this was just done.

neutrinoceros · 2021-01-21T20:26:09Z

I take a bet that the errors on Jenkins are not reproducible.
@yt-fido test this please.

Xarthisius · 2021-01-21T20:49:45Z

I take a bet that the errors on Jenkins are not reproducible.
@yt-fido test this please.

Did you actually try? I had no problem reproducing them...

xarth@shakuras ~/codes/xarthisius/yt (index_chunksize) $ python3.8 doc/source/cookbook/particle_filter.py
yt : [INFO     ] 2021-01-21 14:48:52,674 Parameters: current_time              = 20.100000000000044
yt : [INFO     ] 2021-01-21 14:48:52,679 Allocating for 3.154e+05 particles
Traceback (most recent call last):
  File "doc/source/cookbook/particle_filter.py", line 47, in <module>
    ds = yt.load(filename)
  File "/home/xarth/codes/xarthisius/yt/yt/loaders.py", line 95, in load
    return candidates[0](fn, *args, **kwargs)
  File "/home/xarth/codes/xarthisius/yt/yt/frontends/tipsy/data_structures.py", line 103, in __init__
    super(TipsyDataset, self).__init__(
  File "/home/xarth/codes/xarthisius/yt/yt/frontends/sph/data_structures.py", line 34, in __init__
    super(SPHDataset, self).__init__(
  File "/home/xarth/codes/xarthisius/yt/yt/data_objects/static_output.py", line 1873, in __init__
    super(ParticleDataset, self).__init__(
  File "/home/xarth/codes/xarthisius/yt/yt/data_objects/static_output.py", line 263, in __init__
    self._set_derived_attrs()
  File "/home/xarth/codes/xarthisius/yt/yt/frontends/tipsy/data_structures.py", line 231, in _set_derived_attrs
    self.index
  File "/home/xarth/codes/xarthisius/yt/yt/data_objects/static_output.py", line 509, in index
    self._instantiated_index = self._index_class(
  File "/home/xarth/codes/xarthisius/yt/yt/geometry/particle_geometry_handler.py", line 24, in __init__
    super(ParticleIndex, self).__init__(ds, dataset_type)
  File "/home/xarth/codes/xarthisius/yt/yt/geometry/geometry_handler.py", line 44, in __init__
    self._detect_output_fields()
  File "/home/xarth/codes/xarthisius/yt/yt/geometry/particle_geometry_handler.py", line 269, in _detect_output_fields
    pcounts = self._get_particle_type_counts()
  File "/home/xarth/codes/xarthisius/yt/yt/geometry/particle_geometry_handler.py", line 38, in _get_particle_type_counts
    for df in self.data_files:
  File "/home/xarth/codes/xarthisius/yt/yt/geometry/particle_geometry_handler.py", line 58, in data_files
    self._setup_filenames()
  File "/home/xarth/codes/xarthisius/yt/yt/geometry/particle_geometry_handler.py", line 90, in _setup_filenames
    df = cls(self.dataset, self.io, template % {"num": i}, fi, (start, end))
  File "/home/xarth/codes/xarthisius/yt/yt/frontends/tipsy/data_structures.py", line 23, in __init__
    io._update_domain(self)
  File "/home/xarth/codes/xarthisius/yt/yt/frontends/tipsy/io.py", line 254, in _update_domain
    c = min(self.ds.index.chunksize, stop - ind)
  File "/home/xarth/codes/xarthisius/yt/yt/data_objects/static_output.py", line 509, in index
    self._instantiated_index = self._index_class(
  File "/home/xarth/codes/xarthisius/yt/yt/geometry/particle_geometry_handler.py", line 25, in __init__
    self._initialize_index()
  File "/home/xarth/codes/xarthisius/yt/yt/frontends/sph/data_structures.py", line 89, in _initialize_index
    super(SPHParticleIndex, self)._initialize_index()
  File "/home/xarth/codes/xarthisius/yt/yt/geometry/particle_geometry_handler.py", line 155, in _initialize_index
    self.regions = ParticleBitmap(
  File "yt/geometry/particle_oct_container.pyx", line 477, in yt.geometry.particle_oct_container.ParticleBitmap.__init__
    self.left_edge[i] = left_edge[i]
TypeError: 'int' object is not subscriptable

matthewturk · 2021-01-21T20:50:51Z

I'll attend to this this evening, I hope!

…

On Thu, Jan 21, 2021 at 2:50 PM Kacper Kowalik ***@***.***> wrote: I take a bet that the errors on Jenkins are not reproducible. @yt-fido <https://github.com/yt-fido> test this please. Did you actually try? I had no problem reproducing them... ***@***.*** ~/codes/xarthisius/yt (index_chunksize) $ python3.8 doc/source/cookbook/particle_filter.py yt : [INFO ] 2021-01-21 14:48:52,674 Parameters: current_time = 20.100000000000044 yt : [INFO ] 2021-01-21 14:48:52,679 Allocating for 3.154e+05 particles Traceback (most recent call last): File "doc/source/cookbook/particle_filter.py", line 47, in <module> ds = yt.load(filename) File "/home/xarth/codes/xarthisius/yt/yt/loaders.py", line 95, in load return candidates[0](fn, *args, **kwargs) File "/home/xarth/codes/xarthisius/yt/yt/frontends/tipsy/data_structures.py", line 103, in __init__ super(TipsyDataset, self).__init__( File "/home/xarth/codes/xarthisius/yt/yt/frontends/sph/data_structures.py", line 34, in __init__ super(SPHDataset, self).__init__( File "/home/xarth/codes/xarthisius/yt/yt/data_objects/static_output.py", line 1873, in __init__ super(ParticleDataset, self).__init__( File "/home/xarth/codes/xarthisius/yt/yt/data_objects/static_output.py", line 263, in __init__ self._set_derived_attrs() File "/home/xarth/codes/xarthisius/yt/yt/frontends/tipsy/data_structures.py", line 231, in _set_derived_attrs self.index File "/home/xarth/codes/xarthisius/yt/yt/data_objects/static_output.py", line 509, in index self._instantiated_index = self._index_class( File "/home/xarth/codes/xarthisius/yt/yt/geometry/particle_geometry_handler.py", line 24, in __init__ super(ParticleIndex, self).__init__(ds, dataset_type) File "/home/xarth/codes/xarthisius/yt/yt/geometry/geometry_handler.py", line 44, in __init__ self._detect_output_fields() File "/home/xarth/codes/xarthisius/yt/yt/geometry/particle_geometry_handler.py", line 269, in _detect_output_fields pcounts = self._get_particle_type_counts() File "/home/xarth/codes/xarthisius/yt/yt/geometry/particle_geometry_handler.py", line 38, in _get_particle_type_counts for df in self.data_files: File "/home/xarth/codes/xarthisius/yt/yt/geometry/particle_geometry_handler.py", line 58, in data_files self._setup_filenames() File "/home/xarth/codes/xarthisius/yt/yt/geometry/particle_geometry_handler.py", line 90, in _setup_filenames df = cls(self.dataset, self.io, template % {"num": i}, fi, (start, end)) File "/home/xarth/codes/xarthisius/yt/yt/frontends/tipsy/data_structures.py", line 23, in __init__ io._update_domain(self) File "/home/xarth/codes/xarthisius/yt/yt/frontends/tipsy/io.py", line 254, in _update_domain c = min(self.ds.index.chunksize, stop - ind) File "/home/xarth/codes/xarthisius/yt/yt/data_objects/static_output.py", line 509, in index self._instantiated_index = self._index_class( File "/home/xarth/codes/xarthisius/yt/yt/geometry/particle_geometry_handler.py", line 25, in __init__ self._initialize_index() File "/home/xarth/codes/xarthisius/yt/yt/frontends/sph/data_structures.py", line 89, in _initialize_index super(SPHParticleIndex, self)._initialize_index() File "/home/xarth/codes/xarthisius/yt/yt/geometry/particle_geometry_handler.py", line 155, in _initialize_index self.regions = ParticleBitmap( File "yt/geometry/particle_oct_container.pyx", line 477, in yt.geometry.particle_oct_container.ParticleBitmap.__init__ self.left_edge[i] = left_edge[i] TypeError: 'int' object is not subscriptable — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2658 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAVXO2HCBUAP72XITENQ6LS3CHPTANCNFSM4ODPM43A> .

matthewturk · 2021-01-22T14:24:36Z

Oddly enough on my branch (merged with main) this is not the specific traceback I get using load_sample! I suspect there's something with the kwargs.

matthewturk · 2021-01-22T14:26:54Z

When run without load_sample, the bounds are not passed in to the constructor. The difference ends up being whether it searches for the left/right edges or not, and where the error ends up showing up as a result. In both cases, I believe, the left_edge is set somewhere as 0 (not as an array) and this propagates and causes the problem.

matthewturk · 2021-01-22T14:36:08Z

OK, I tracked down the issue. Having chunksize hang off the index is problematic in some cases, because we need to call update_domain. But when that is called, the index is not ready yet.

I'm going to modify the code to use a hardcoded chunksize for the tipsy update_domain section.

Make CHUNKSIZE an index attribute

0ba246c

matthewturk added bug index: particle labels Jun 20, 2020

matthewturk added this to the 4.0 milestone Jun 20, 2020

matthewturk mentioned this pull request Jun 20, 2020

merging yt-4 into master #2437

Merged

munkm changed the base branch from yt-4.0 to master June 22, 2020 15:08

half-finished rename

077c461

matthewturk mentioned this pull request Jun 29, 2020

[BUGFIX] Art: fix dm_only frontend #2688

Merged

neutrinoceros approved these changes Jul 19, 2020

View reviewed changes

matthewturk added 2 commits July 19, 2020 14:02

Merge branch 'master' into index_chunksize

499ce82

Changing to a property

2519f0f

neutrinoceros approved these changes Jul 19, 2020

View reviewed changes

Merge branch 'master' into index_chunksize

9b79f01

Fix black manually

20a37e4

matthewturk force-pushed the index_chunksize branch from 8904cb1 to 9b79f01 Compare September 18, 2020 15:11

neutrinoceros reviewed Sep 18, 2020

View reviewed changes

brittonsmith approved these changes Nov 18, 2020

View reviewed changes

Base automatically changed from master to main January 20, 2021 15:27

Merge branch 'main' of github.com:yt-project/yt into index_chunksize

9891b82

Using hardcoded local chunksize for tipsy update domain code

e4d4d68

brittonsmith merged commit aeb4947 into yt-project:main Jan 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make CHUNKSIZE into an index property #2658

Make CHUNKSIZE into an index property #2658

matthewturk commented Jun 20, 2020 •

edited

Loading

matthewturk commented Jun 23, 2020

Xarthisius commented Jun 29, 2020

neutrinoceros left a comment

matthewturk commented Jul 19, 2020

neutrinoceros commented Jul 19, 2020

neutrinoceros left a comment

matthewturk commented Sep 16, 2020

matthewturk commented Sep 18, 2020

neutrinoceros commented Sep 18, 2020

matthewturk commented Sep 18, 2020

neutrinoceros commented Sep 18, 2020

matthewturk commented Sep 18, 2020

neutrinoceros left a comment

neutrinoceros commented Sep 18, 2020

neutrinoceros commented Sep 18, 2020

Xarthisius commented Jan 21, 2021

brittonsmith commented Jan 21, 2021 •

edited

Loading

neutrinoceros commented Jan 21, 2021

Xarthisius commented Jan 21, 2021

matthewturk commented Jan 21, 2021 via email

matthewturk commented Jan 22, 2021

matthewturk commented Jan 22, 2021

matthewturk commented Jan 22, 2021

Make CHUNKSIZE into an index property #2658

Make CHUNKSIZE into an index property #2658

Conversation

matthewturk commented Jun 20, 2020 • edited Loading

matthewturk commented Jun 23, 2020

Xarthisius commented Jun 29, 2020

neutrinoceros left a comment

Choose a reason for hiding this comment

matthewturk commented Jul 19, 2020

neutrinoceros commented Jul 19, 2020

neutrinoceros left a comment

Choose a reason for hiding this comment

matthewturk commented Sep 16, 2020

matthewturk commented Sep 18, 2020

neutrinoceros commented Sep 18, 2020

matthewturk commented Sep 18, 2020

neutrinoceros commented Sep 18, 2020

matthewturk commented Sep 18, 2020

neutrinoceros left a comment

Choose a reason for hiding this comment

neutrinoceros commented Sep 18, 2020

neutrinoceros commented Sep 18, 2020

Xarthisius commented Jan 21, 2021

brittonsmith commented Jan 21, 2021 • edited Loading

neutrinoceros commented Jan 21, 2021

Xarthisius commented Jan 21, 2021

matthewturk commented Jan 21, 2021 via email

matthewturk commented Jan 22, 2021

matthewturk commented Jan 22, 2021

matthewturk commented Jan 22, 2021

matthewturk commented Jun 20, 2020 •

edited

Loading

brittonsmith commented Jan 21, 2021 •

edited

Loading