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.
Summary
rotationmap.fit_mapdocuments (anddefault_parameters.ymldeclares) a pressure-correctedrotation-curve option via
r_pressure/w_pressure, backed by working_vkep_pressure/_vpow_pressureimplementations. Butverify_params_dictionarynever selects them —params['vfunc']is unconditionally set to
_vpowor_vkep. Settingr_pressurecurrently has no effect onthe fitted model.
Version
eddy 3.1.1,
eddy/rotationmap.py.Where
There is no branch checking
params['r_pressure']here, so_vkep_pressure(rotationmap.py:1579)and
_vpow_pressure(rotationmap.py:1605) are unreachable fromfit_map's public API, despitebeing actively maintained — see the
jnp.where-vs-Python-iftracing comments in both functions,added to fix a
TracerBoolConversionErrorunder the JIT/vmap path.Regression
The selection logic used to exist. Commit
4631f4d("included pressure term for outer disk",2021-09-24) added it:
It was dropped in commit
5e336b5("adding option for self-gravity", 2022-06-06), which rewroteverify_params_dictionaryaround the new disk-self-gravity (mdisk) feature and did not carry thehas_pressurebranch forward.r_pressure/w_pressurehave remained indefault_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
Expected vs. actual
r_pressure(and optionallyw_pressure) is set,fit_mapfits thepressure-corrected rotation curve (
_vkep_pressure/_vpow_pressure).vfuncalways resolves to the plain_vkep/_vpow;r_pressure/w_pressureare silently ignored.
Suggested fix
In
verify_params_dictionary, branch onparams['r_pressure'] is not Nonethe way4631f4doriginally did, selecting
_vkep_pressure/_vpow_pressureinstead of_vkep/_vpow. Add aregression test (
tests/test_rotationmap.pyhas no coverage forr_pressurecurrently) so thiscan't silently regress again.