incorperate image homology into loop matching#97
Conversation
stanfish06
commented
Jul 13, 2026
stanfish06
commented
- landscape analysis
- landscape
- landscape
- add downsample mode and cleanup code
- prepare for image based cross species matching
- make ripser return birth and death simplices
- update: image based filtration
- fix: downsample logic
- update: image based rips
- update: image based loop matching for bootstrap
- fix: memory issue for image homology
- fix bug: diagonal entry handling for image PH
- update readme
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b5b1e3e31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| ############ | ||
|
|
||
| from match.utils_plot import * |
There was a problem hiding this comment.
In any environment that imports scloop.matching.utils, this top-level import raises ModuleNotFoundError before any of the new matching helpers can be used; I searched the repo for utils_plot/match and this is the only reference, so the dependency is neither vendored nor package-relative.
Useful? React with 👍 / 👎.
| return out | ||
|
|
||
| #### define your system | ||
| send_cmd = send_cmd_windows |
There was a problem hiding this comment.
When this module is used on Linux or macOS, helpers like compute_image_bars still call send_cmd_windows because of this unconditional assignment. That runner passes the entire command string to subprocess.Popen without shell=True, so POSIX tries to execute a literal filename like ./ripser-image --dim ... instead of running the command; choose the runner based on the platform or pass argv explicitly.
Useful? React with 👍 / 👎.
| else | ||
| columns_to_reduce.push_back(edge); |
There was a problem hiding this comment.
When computing image homology on the union complex, sub_edges still contains ambient edges whose endpoints are not both in the chosen subfiltration, with diameter_sub == inf. This branch can enqueue those ambient-only edges as reduction columns, allowing them to claim pivots before true subfiltration edges and thereby suppress or shift H1 image bars whenever the two samples are connected through cross/other-sample edges; the column-building pass should skip edges whose subfiltration diameter exceeds the threshold, as the dim-0 pass above already does.
Useful? React with 👍 / 👎.
| size=len(source_indices) | ||
| if bootstrap_sampling == "resample" | ||
| else int(len(source_indices) * bootstrap_downsample_fraction), |
There was a problem hiding this comment.
When bootstrap_sampling="downsample", this uses int(len(source_indices) * bootstrap_downsample_fraction) directly. For small inputs or low fractions this can choose fewer than two vertices, e.g. two source vertices with the default 2/3 fraction produces a one-point bootstrap, unlike the FPS branches that clamp to at least two; downstream homology rounds then fail or produce unusable empty loop samples instead of a valid bootstrap.
Useful? React with 👍 / 👎.
| # Extract data from self based on bootstrap flag | ||
| if not bootstrap: | ||
| assert self.persistence_diagram is not None | ||
| assert self.persistence_pair_simplices is not None |
There was a problem hiding this comment.
After save_scloop/load_scloop, persistence_pair_simplices is always None because the HDF5 writer only serializes persistence_diagram and cocycles; this new assertion therefore makes re-running loop representative computation on a loaded homology object fail even though the diagrams and cocycles needed by the previous implementation were persisted. Either serialize/deserialize the pair simplices or allow this call to proceed with None for backward-compatible loaded data.
Useful? React with 👍 / 👎.
1 participant
