Open kyleaoman opened 1 year ago
thanks for your work on this! so to be clear, what you would want is the max extent of any particle (parttypes 0-->5) from the galaxy/halo center? or just some specific particles?
I would be ok with either one because the subregions in swiftsimio are usually fairly coarse and the workflow is to read a refion that is inevitably somewhat larger than what is actually desired then remove some particles. So in practice the I/O load would usually be the same whether I used the overall maximum or tried to iterate over particle types. Of course I guess it's cheap to include all of them, and I can easily do max(smax, dmmax, ax bhmax)
.
Alternatively you could also choose to specify the bounding box of the group. Something like (min_x, max_x, min_y, max_y, min_z, max_z)
. In principle this is conceptually exactly the measurement that I need, but in practice using the box with sides of length 2max_r
centred on the group centre is always guaranteed to contain the other one, and only rarely leads to a very slight loss in efficiency.
On further reflection, the bounding box variant would have some advantages as it would make cases where the periodic boundary is contained in the box easier to detect. To be clear if the box has size 1.0 then a bounding box such as (0.9, 0.1, 0.9, 0.1, 0.9, 0.1) would have a side length of 0.2 and be centred in the (0,0,0) corner of the box, or the (1,1,1) corner - of course it's the same place. It's possible to reconstruct the bounding box from just a radius and detect whether it crosses a periodic boundary anyway, but requires a couple of extra lines of logic. So still up to you but I guess now I have a slight preference for one option :)
hey - sorry i fell asleep on this issue/feature request. JWST proposals are starting to sink me...
is this commit by @romeeld fixing this issue?
1a84b0d79e262a34ad3ccfc262c3ab5df5ac734a
That's the intent. It currently breaks caesar installation on the swiftgalaxy CI tests. I'll make sure that everything is working as wanted from my side and let you know when this is all resolved - hopefully soon.
Looks like the build and installation is working fine now, thanks for those fixes. Going to try and make sure the addition of the rmax fields is providing what it needs to on my end now.
This is a feature request to optimize integration of Caesar into
swiftgalaxy
.That package facilitates retrieving a collection (e.g. halo, galaxy) of particles from a SWIFT snapshot and providing them in a class with various utilities to manipulate them. After chatting with Romeel, I've added integration of Caesar in the branch linked above, but currently it's difficult to implement use of swiftsimio's sub-region reading robustly because I don't know the total extent of the particle group. Could you add another radius to the ones already available that specifies the maximum radius of any particle in the group?