Update TNG tests and add another dont_cache check for loading #2624
Conversation
|
So this also updates the TNG Halo test to use a bounding box that includes particles. I'll change the name. |
matthewturk
changed the title
|
@AshKelly I added you to review this since you've been thinking about this too. |
| dont_load = dont_cache and not hasattr(ds, 'index_filename') | ||
| try: | ||
| rflag = self.regions.load_bitmasks(fname) | ||
| rflag = self.regions.check_bitmasks() | ||
| self._initialize_frontend_specific() | ||
| if rflag == 0: | ||
| if dont_load or rflag == 0: | ||
| raise OSError |
There was a problem hiding this comment.
If I follow this: in the case that we don't want to load, dont_load = True we still try and load the bitmasks, but then we raise an OSError which triggers the exception and thus, we then initialize from scratch?
If so, I guess this is ok, because loading is fast.
There was a problem hiding this comment.
That was the idea, yeah, but I think maybe it's not the best idea anymore. It might need to be worked through a bit more carefully.
There was a problem hiding this comment.
I guess you could put the if dont_load: raise OSError straight after the try and then at the end of the try have the if flag == 0: raise OSError - otherwise yt outputs:
Loading particle index...
Generating particle index...
There was a problem hiding this comment.
good point, will do this
|
Updated as per @AshKelly 's suggestion |
made domain_override private Co-authored-by: Matthew Turk <matthewturk@gmail.com>
matthewturk
force-pushed
the
dont_cache_onload
branch
from
9e2e2d6 to
ce427fa
Compare
This is the next step after #2618, with one remaining. This checks whether or not an
index_filenamehas been provided; if it has, it overrides thedont_cachefor whether we should load from the files.This covers the case that we aren't caching, but an index file exists, and it also covers for if an index file exists and is specified. It's not yet possible to create an index file since
dont_cachedoesn't checkindex_filename.Tests for this will be forthcoming, once I've stabilized the yt4-merge in #2437 and the Arepo tests there.