Skip to content

Conversation

@VictorForouhar
Copy link
Collaborator

@VictorForouhar VictorForouhar commented Oct 1, 2025

This PR will add rotation velocity calculations for subhalo aperture properties, as we may not want to use all bound stellar particles. Additionally, I will add luminosity-weighted averages for rotation velocities and velocity dispersions.

TODO:

  • Mass-weighted rotation velocity calculation for aperture properties
  • Mass-weighted cylindrical velocity dispersion calculation for aperture properties
  • Luminosity-weighted implementation:
    • Define cylindrical velocities for each luminosity-weighted angular momentum vector (as they may differ across bands)
    • Luminosity-weighted rotational velocity.
    • Luminosity-weighted velocity dispersion tensor.

@VictorForouhar
Copy link
Collaborator Author

Managed to run the new functions successfully on an example from COLIBRE box, but it only has 15 stellar particles. I will test on a bigger box.

Also, I get negative rotation velocities for some of the bands, which is something I will also look into it.

We do not do so when computing the angular momentum vector, so this change makes it consistent with our choice to not re-centre.
Including the case for when we do luminosity-weighting. This reflects the same choice we made when computing luminosity-weighted angular momenta.
@VictorForouhar
Copy link
Collaborator Author

The latest changes make consistent choices about velocity/position centering across angular momentum and cylindrical coordinate calculation (including mass-weighted).

After these changes, I no longer get a negative velocity for the example I was investigating (it went from a rotational velocity of -12km/s to 52km/s; quite a large change!)

@VictorForouhar
Copy link
Collaborator Author

I have been using this branch for L200m6 and L400m7 without crashes. Also for the L25 Mpc volumes across different resolutions.

Copy link
Collaborator

@robjmcgibbon robjmcgibbon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching the inconsistent reference positions and velocities. Can you please update the descriptions of the properties that changed? Then good to merge!

@VictorForouhar VictorForouhar merged commit 10b59c3 into master Oct 27, 2025
1 check passed
@VictorForouhar VictorForouhar deleted the rotation_velocity_changes branch October 27, 2025 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants