Coordinate transformation: hyperpipe#133
Open
oshaughnessy-junior wants to merge 4 commits into
Open
Conversation
Until now --supplementary-coordinate-code only accepted the literal string
'rift_default'. Extend it into a real plugin mechanism so users can fit
and sample in arbitrary coordinate systems without editing the driver.
* New RIFT.misc.coordinate_plugin module documents the plugin contract
and provides load_coordinate_converter(). The loader accepts three
forms of --supplementary-coordinate-code: the literal 'rift_default'
(now routed through a built-in shim around convert_waveform_coordinates
+ the canonical rift_priors.prior_map), a filesystem path to a .py
file (loaded via importlib.util.spec_from_file_location), or any
importable dotted module name.
* Plugin contract. A plugin exposes at minimum a callable
convert_coordinates(x_in, coord_names, low_level_coord_names, **kwargs)
returning a 2-D ndarray of shape (N, len(coord_names)). Optional
hooks: prepare(config, coord_names, low_level_coord_names, chart,
opts, **kw) (one-shot setup, gets the parsed ini),
register_priors(prior_map, ...) (in-place mutation), and a documented-
but-not-yet-consumed jacobian() slot so plugin authors and future RIFT
code converge on one signature.
* CHARTS abstraction. RIFT's separable per-name prior_map cannot
represent "the prior on y in (x,y,z)" and "the prior on y in (r,y,z)"
simultaneously -- the implicit prior depends on the whole chart. A
plugin may therefore declare CHARTS = {name: {parameters, priors,
ranges, ...}}; each chart owns its own separable {name: callable}
prior dict, so the same coordinate label in two charts no longer
collides on a single prior_map key. The user selects one with
--supplementary-coordinate-chart NAME. Resolution priority is
explicit > DEFAULT_CHART > sole entry > error. The loader installs
CHARTS[chart]['priors'] into prior_map and any CHARTS[chart]['ranges']
into prior_range_map (only for names the user hasn't already pinned
via --integration-parameter-range).
* New CLI flags on util_ConstructEOSPosterior.py:
--supplementary-coordinate-function NAME (default
'convert_coordinates')
--supplementary-coordinate-ini PATH (parsed and handed to prepare())
--supplementary-coordinate-chart NAME (selects an entry in CHARTS)
The 'rift_default' path keeps working, now flowing through the loader
instead of being special-cased in the driver.
* Reference plugin in demo/hyperpipe/linear_coordinate_convert.py
implements y = A.x + b with separable uniform priors on the rotated
basis, sized to the existing 3-D Gaussian demo grid. The companion
linear_coordinate_convert.ini holds A, b, and the input / output
name lists (JSON-encoded matrices to avoid inventing another number
grammar).
Verified end-to-end:
- linear demo: row [2,3,5] -> (3.5355, 0.7071, 5.0); column-permutation
invariance on both input and output sides; missing ini / coord-name
mismatch / missing file raise clean errors with actionable messages.
- single-chart auto-select installs priors and ranges; user-set ranges
survive (chart ranges only fill unset names).
- multi-chart plugin: without --supplementary-coordinate-chart
selection -> ValueError listing available charts; selecting 'xyz'
vs 'ryz' on the same plugin installs DIFFERENT priors under the
same key 'y', confirming the chart namespace disambiguates
correctly.
- --supplementary-coordinate-chart on a plugin without CHARTS warns
and proceeds; bogus chart name raises with the available list.
Companion to the previous commit ("util_ConstructEOSPosterior: pluggable
coordinate-convert code"), exercising the linear-coordinate-convert plugin
through the hyperpipe pipeline.
example_gaussian_uvw.py
-----------------------
Drop-in replacement for example_gaussian.py with the same CLI surface.
The likelihood is a multivariate Gaussian centred at (u, v, w) =
(0, 4.95, 3.5) with deliberately unequal sigmas (0.5, 1.0, 1.5) so the
rotation is visible in the recovered (x, y, z) posterior -- a unit move
along z (broad axis sigma_w=1.5) costs less lnL than a unit move along x
(which mostly projects onto the narrow sigma_u=0.5).
The evaluator imports linear_coordinate_convert.py by file path the same
way RIFT.misc.coordinate_plugin.load_coordinate_converter does -- i.e.
through the canonical plugin contract -- and feeds each grid row through
convert_coordinates(..., chart="uvw_rotated") before evaluating the
Gaussian. This demonstrates that the same plugin library is consumable
from both the post stage (via --supplementary-coordinate-code) and from
arbitrary user evaluator code.
Mean and sigmas are user-tunable via --mu-uvw / --sigma-uvw so the same
script can drive different demos. Plugin and ini paths default to
sibling files in the script's directory and are overridable via
--coord-plugin / --coord-ini.
Also corrects an off-by-one inherited from example_gaussian.py: the
original clamps eos_end_index >= len(eoss) to len(eoss)-1 and then
slices [start:end], silently dropping the last row of the grid. The
new version uses Pythonic half-open semantics so every requested row is
evaluated and written.
hyperpipe_conf_linear_uvw.yaml
------------------------------
Variant of hyperpipe_conf_tracer.yaml that swaps in example_gaussian_uvw.py
as the marg evaluator. Iteration stays in (x, y, z): the yaml schema
currently shares `coords-fit` between the puff and post stages (see
RIFT/hyperpipe/coords.py:to_puff_args / to_post_args), so having puff
operate in one basis and the EOS posterior fit in another would need a
schema extension; that's deliberately out of scope here. The post
stage's --supplementary-coordinate-code path is covered by the unit
tests in RIFT/misc/coordinate_plugin.py.
The marg-list args line uses Hydra's ${hydra:runtime.cwd} resolver to
ship absolute plugin / ini paths to the condor scratch dir, so the
marg job finds them regardless of where condor schedules it.
Integration ranges in (x, y, z) are sized to comfortably enclose
~3 sigma around the Gaussian's back-projected centre.
Verified end-to-end on a synthetic 5-row (x, y, z) grid: lnL values
match the analytic Gaussian to 1e-4, the peak lands at the row that
maps exactly to (0, 4.95, 3.5) in (u, v, w), and the per-row breakdown
shows the unequal-sigma signature as expected (row 2: u-axis-dominated
hit; row 3: w-axis-dominated, much smaller).
Run with:
util_RIFT_hyperpipe.py --config ./hyperpipe_conf_linear_uvw.yaml
Then plot the result from rundir_linear_uvw/ with plot_posterior_corner.py
on posterior-4.dat -- the recovered ellipsoid in (x, y, z) should point
along the rotated v-axis, with its narrowest extent along the u-axis.
Symptom
-------
After a hyperpipe run, test.sub came out with a bare-name executable
line:
executable = convergence_test_samples
even though sibling sub files (e.g. EOS_POST_worker.sub) correctly
carried the absolute path:
executable = /home/.../bin/util_ConstructEOSPosterior.py
Condor on hosts that do not search PATH for local-universe jobs then
failed to find the script.
Root cause
----------
The installed script is named convergence_test_samples.py (setup.py
ships bin/* verbatim, with the .py suffix preserved). Four upstream
layers, however, defaulted to the bare name `convergence_test_samples`
and handed that down into the path-resolution step at
create_eos_posterior_pipeline:453-457:
test_exe_resolved = opts.test_exe
if test_exe_resolved and not os.path.isabs(test_exe_resolved):
_r = dag_utils.which(test_exe_resolved)
if _r:
test_exe_resolved = os.path.abspath(_r)
dag_utils.which walks PATH for the literal filename it was given. No
file named `convergence_test_samples` (no extension) exists on PATH, so
which() returned None and the conditional silently left
test_exe_resolved as the bare name -- which is what landed in test.sub.
The .py-suffixed fallback at the dag_utils write_test_sub layer never
got a chance to run, because callers were always handing it an explicit
(wrong) exe argument.
Fix
---
Layered so both the in-tree defaults and any user yaml that still has
the bare-name string get absolutized correctly:
* create_eos_posterior_pipeline: the existing which-based resolver
now retries with a `.py` suffix when the bare lookup misses, and
prints a clear warning if neither form resolves. User configs that
say `exe: convergence_test_samples` keep working without edits.
* Argparse default in the same file changed from
'convergence_test_samples' to 'convergence_test_samples.py' so the
in-tree default matches the installed filename.
* util_RIFT_hyperpipe.py: yaml-fallback default likewise updated, with
a comment explaining why the .py is required for the downstream
which() to succeed.
* RIFT/hyperpipe/config.py: DEFAULT_CONFIG_YAML's test.exe field
updated, with an inline comment so future readers know why.
* bin/hyperpipe_conf.yaml: same update.
Verified
--------
Synthetic test against a fake PATH directory containing only
convergence_test_samples.py:
- bare-name input -> /tmp/fakebin/convergence_test_samples.py
- .py-suffixed input -> /tmp/fakebin/convergence_test_samples.py
- absolute-path input -> /tmp/fakebin/convergence_test_samples.py
- missing exe -> preserved as-is + warning emitted
The existing hydra-integration test
(test/hyperpipe/tests/test_hydra_integration.py:118) does a substring
check on `--test-exe convergence_test_samples`; the new
`.py`-suffixed default still contains that substring, so no test
update is required.
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.
Basic coordinate module, stage 0