Add radial velocity component to the fit_map rotation curve - #38
Open
richteague wants to merge 1 commit into
Open
Add radial velocity component to the fit_map rotation curve#38richteague wants to merge 1 commit into
richteague wants to merge 1 commit into
Conversation
vr_100/vr_q were declared in default_parameters.yml with priors but never wired into _make_model, so the global parametric fit had no way to include a radial (infall/wind) velocity term -- unlike fit_annuli, which already supports fit_vrad. Restore this by projecting _vrad(r) via _proj_vrad and adding it into v0 whenever vr_100 is set. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
vr_100/vr_qwere declared indefault_parameters.ymlwith priors but never wired into_make_model, sofit_maphad no way to fit a radial (infall/wind) velocity term on top of the rotation curve -- unlikefit_annuli, which already supports this viafit_vrad._vrad(r)(power-law radial velocity), aparams['vradial']flag set fromvr_100 is not None, and project/add it intov0in_make_modelalongside the existing rotational, vortex, and vlsr terms.vr_100) is unchanged -- verified byte-identical model output.Test plan
test_make_model_includes_radial_velocity-- confirmsvr_100/vr_qperturb_make_model's output by the expectedvr_100 * sin(inc)projection, and that omittingvr_100reproduces the prior model exactly.pytest tests/)ruff checkclean