Conversation
There was a problem hiding this comment.
Pull request overview
This PR completes previously-missed “drawing improvements” work by stabilizing the constrained x-layout used for Reeb/Mapper drawing and adding regression tests around layout determinism and y-coordinate invariants.
Changes:
- Stabilize
reeb_x_layoutby avoiding unbounded drift on edgeless graphs, adding bounds, and adding a small centering regularizer. - Add new unit tests for layout behavior (finite/normalized x, seed reproducibility) and for
set_pos_from_fpreserving y-values. - Bump package version to
0.1.14and add/update documentation images.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
cereeberus/cereeberus/draw/layout.py |
Adds normalization helper, handles edgeless graphs, introduces bounded optimization and a centering term. |
tests/test_draw_layout.py |
New regression tests for reeb_x_layout finiteness, normalization range, reproducibility, and edgeless cases. |
tests/test_reeb_class.py |
Adds test ensuring set_pos_from_f preserves y == f(v); adjusts imports. |
tests/test_mapper_class.py |
Adds test ensuring set_pos_from_f preserves y == delta * f(v); adjusts imports. |
pyproject.toml |
Version bump 0.1.13 → 0.1.14. |
doc_source/images/torus-extraverts.png |
Adds/updates rendered documentation image asset. |
doc_source/images/line.png |
Adds/updates rendered documentation image asset. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| unittest.main() | ||
| if __name__ == '__main__': |
There was a problem hiding this comment.
There are two identical if __name__ == '__main__': unittest.main() blocks at the end of the file, which will run the suite twice when executed directly. Remove the duplicate so there is only a single entry point.
| unittest.main() | |
| if __name__ == '__main__': |
| unittest.main() | ||
| if __name__ == '__main__': |
There was a problem hiding this comment.
There are two identical if __name__ == '__main__': unittest.main() blocks at the end of the file, which will run the suite twice when executed directly. Remove the duplicate so there is only a single entry point.
| unittest.main() | |
| if __name__ == '__main__': |
| @@ -146,6 +168,7 @@ def energy(x): | |||
| for i, j in same_height_pairs: | |||
| diff = x[i] - x[j] | |||
| e += repulsion / (diff**2 + 1e-6) | |||
| e += center_weight * np.dot(x, x) | |||
There was a problem hiding this comment.
The module docstring at the top defines the layout energy without the new centering term, but the implementation now adds center_weight * dot(x, x) (and bounds). Please update the documented energy expression/description so it matches the optimizer objective.
Some stuff didn't get pushed on the last PR, fixing that.