Skip to content

fit_map: r_pressure/w_pressure pressure-correction never selected — vfunc always resolves to _vkep/_vpow #36

Description

@richteague

Summary

rotationmap.fit_map documents (and default_parameters.yml declares) a pressure-corrected
rotation-curve option via r_pressure/w_pressure, backed by working _vkep_pressure /
_vpow_pressure implementations. But verify_params_dictionary never selects them — params['vfunc']
is unconditionally set to _vpow or _vkep. Setting r_pressure currently has no effect on
the fitted model.

Version

eddy 3.1.1, eddy/rotationmap.py.

Where

# eddy/rotationmap.py, verify_params_dictionary, ~L1229
if params['vp_100'] is not None:
    if params['mstar'] is not None:
        params['vfunc'] = self._vpow
    else:
        raise ValueError("Cannot specify both `vp_100` and `mstar`.")
else:
    params['vfunc'] = self._vkep

There is no branch checking params['r_pressure'] here, so _vkep_pressure (rotationmap.py:1579)
and _vpow_pressure (rotationmap.py:1605) are unreachable from fit_map's public API, despite
being actively maintained — see the jnp.where-vs-Python-if tracing comments in both functions,
added to fix a TracerBoolConversionError under the JIT/vmap path.

Regression

The selection logic used to exist. Commit 4631f4d ("included pressure term for outer disk",
2021-09-24) added it:

has_pressure = False if params.get('r_pressure') is None else True
...
if has_mstar:
    if has_pressure:
        params['vfunc'] = self._proj_vkep_pressure
    else:
        params['vfunc'] = self._proj_vkep

It was dropped in commit 5e336b5 ("adding option for self-gravity", 2022-06-06), which rewrote
verify_params_dictionary around the new disk-self-gravity (mdisk) feature and did not carry the
has_pressure branch forward. r_pressure/w_pressure have remained in default_parameters.yml
(current lines 143, 150) and the underlying velocity functions have kept being fixed (most recently
for JAX tracer correctness) ever since, so this reads as an accidental drop rather than an intentional
deprecation.

Reproduce

# rmap = some rotationmap with a fittable map
params = {'mstar': 0, 'vlsr': 1, 'r_pressure': 50.0, 'w_pressure': 10.0}
verified = rmap.verify_params_dictionary(params.copy())
print(verified['vfunc'].__name__)   # '_vkep'  -- not '_vkep_pressure'

Expected vs. actual

  • Expected: when r_pressure (and optionally w_pressure) is set, fit_map fits the
    pressure-corrected rotation curve (_vkep_pressure/_vpow_pressure).
  • Actual: vfunc always resolves to the plain _vkep/_vpow; r_pressure/w_pressure
    are silently ignored.

Suggested fix

In verify_params_dictionary, branch on params['r_pressure'] is not None the way 4631f4d
originally did, selecting _vkep_pressure/_vpow_pressure instead of _vkep/_vpow. Add a
regression test (tests/test_rotationmap.py has no coverage for r_pressure currently) so this
can't silently regress again.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions