-
Notifications
You must be signed in to change notification settings - Fork 308
Description
(This is more of a question than a bug, so feel free to close)
The procedure for adding new Rockstar revisions seems relatively straightforward for versions that are a pure/"official" update:
- Add revision to list of known revisions:
- KNOWN_REVISIONS: list[int] = [0, 1, 2] + KNOWN_REVISIONS: list[int] = [0, 1, 2, 3]
- Add/modify fields to match
halo_dt: list[HaloDataType] = [ ("particle_identifier", np.int64), ("particle_position_x", np.float32), ... - ("particle_corevel_z", np.float32, (1, 100)), #Example modification/removal + ("particle_corevel_z", np.float32, (1, 2)), ... + ("new_halo_field", np.float32, (3, 100)), #Example addition ]
What is the procedure, however, if the revision is effectively an alternate of one of the current revisions? E.g. a Rockstar dataset with all of the fields from revision 1, none of the fields from revision 2, and additional fields? Just call it revision 4 and follow the procedure above?
For context/example, I primarily use a fork of Rockstar from Andrew Wetzel (here), which follows revision 2, but has two additional fields (m_hires and m_lowres). So I'd follow steps 1 & 2 above:
KNOWN_REVISIONS: list[int] = [0, 1, 2, 3] # Added version 3
...
halo_dt: list[HaloDataType] = [
("particle_identifier", np.int64),
...
("av_density", np.float32, (2, 100)),
("m_hires", np.float32, (3, 100)), # New
("m_lowres", np.float32, (3, 100)), # New
]But Andrew also has another fork (alt fork) which is based off of revision 1, with an additional field, halfmass_radius, between m_pe_d and num_p. So my options seem to be
- I could add
halfmass_radiiwith the version gate (1, 1) or similar, but then there'd be problems if I use, say, Behroozi's original version for something. - I could call it version 4, version gate all the fields introduced in revision 2 and add the new field, but that doesn't seem very resilient to future changes
- I could call it version 1.x (1.1, 1.5, etc) and add
("halfmass_radii", np.float32, (1.x, 1.x)),. This seems like the best option, but what happens if a later revision adds the field as well? - Other?
Which one is preferred?
Corollary, would PR's for those changes be desired, even if they potentially conflict (e.g. like the alt fork above)?
I originally asked this question in #4997, but I've realized that's the wrong venue and am hoping to move the discussion here.