SWIFTSIM / swiftsimio

Python library for reading SWIFT data. Uses unyt and h5py.
GNU Lesser General Public License v3.0
15 stars 12 forks source link

Allows file handles to stick around longer #155

Closed JBorrow closed 9 months ago

JBorrow commented 1 year ago

Attempted remedy to #153.

This stores a file handle in SWIFTUnits, which is accessed by SWIFTMetadata. It is accessed through a property, meaning that the file can be re-opened if necessary.

@bwvdnbro Is it possible to test this (with peter's blessing?)

If we need to go further, we can stick the file handle into the dataset reading. But those should take long enough that grabbing file handles is ok.

bwvdnbro commented 1 year ago

@JBorrow I'm not sure what I would test here. I can check that this works, but I don't really have a good way of confirming if this helps with the unusually large load on the meta-data server. Peter telling me off is not exactly a very good diagnostic. I also expect that it will be hard to get hold of 192 nodes at the same time again. One would hope that the queue is usually busy enough to prevent that.

I don't think it can do any harm to open the file less though.

JBorrow commented 1 year ago

Would it be possible to run the morphology pipeline to make sure nothing is messed up?

MatthieuSchaller commented 10 months ago

What is the status of this? Do we still want the change? If so what can we do to test it?

JBorrow commented 10 months ago

I would like to see somebody run this at scale and see if it breaks anything. It is a generally good change, and does not break anything. For that reason we can probably just merge it in and see if anyone complains...

MatthieuSchaller commented 10 months ago

@jchelly or @kyleaoman may have large test cases.

kyleaoman commented 9 months ago

I don't think that I do...

JBorrow commented 9 months ago

Well it's not destructive and just seems to be a positive change, so I am merging it.