Refactor/plume mesh consolidation pr116 - #126
Merged
andytorrestb merged 5 commits intoJul 26, 2026
Merged
Conversation
stl.py defined transform_mesh twice: the reusable in-memory API transform_mesh(mesh_obj, rotation_matrix=..., translation_vector=..., scale_factor=...) and, later in the file, a file-based CLI helper transform_mesh(input_file, output_file, scale, translate). The second definition shadowed the first on import, so callers that passed a mesh object plus rotation_matrix/scale_factor (e.g. graph_jfh_thruster_check) would have raised TypeError. Keep the mesh-object API as transform_mesh; rename the CLI/file-based helper to _transform_stl_file_cli and update its only caller (the __main__ block). Scaling/translation/save/print behavior is unchanged. Add compose_meshes(): the canonical home for generic mesh concatenation (ordered, single np.concatenate, one mesh returned unchanged, raises on zero/None inputs, never mutates inputs). Replaces incremental np.concatenate blocks duplicated across the study methods. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Make VisitingVehicle.transform_plume_mesh the canonical owner of plume
placement geometry. Extend it (not a separate PlumeMeshUtils module) with
optional vv_orientation/vv_position so it supports both:
* local thruster placement (no pose): thruster DCM + thruster exit only
-- the historical behavior used by check_thruster_configuration and
LogisticsModule.plot_thruster_group; no cluster offset; and
* complete vehicle/JFH placement (pose supplied): thruster DCM, VV
orientation, VV position, cluster exit offset, thruster exit -- the
legacy graph_jfh/visualize_sweep sequence, in the exact same order.
Callers pass DCMs exactly as stored; transposition/sequencing is handled
inside the method. Mutation semantics are preserved (numpy-stl mutates in
place). Light shape validation (3x3 DCMs, length-3 vectors) fails fast on
malformed config data without affecting valid legacy inputs.
Add _cluster_id_for_thruster: parse the full leading P<n> prefix so
multi-digit clusters (P10T1 -> P10) resolve correctly while single-digit
ids behave identically to the legacy first-two-character parse. Missing
required clusters raise a descriptive KeyError (never silently omitted).
Add cached get_thruster_id_map()/get_thruster_id(): JFH 1-based index ->
canonical thruster id, preserving thruster_data insertion order, built
lazily and invalidated when set_thruster_config() replaces the config.
Kept on VisitingVehicle, not the base Vehicle class.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
graph_jfh, visualize_sweep, and graph_jfh_thruster_check each rebuilt the
JFH index->name map, inlined the same plume placement sequence, and did
incremental np.concatenate composition. Replace those with the centralized
primitives:
* thruster id resolution via vv.get_thruster_id (cached map);
* plume placement via vv.transform_plume_mesh (complete placement for
graph_jfh/visualize_sweep; local placement for the thruster check, so
the thruster DCM/exit are applied exactly once -- no double-apply);
* composition via compose_meshes (active cones, cluster meshes, and the
final VV body + cones + optional clusters).
Behavior is preserved exactly: same transform order, same concatenation
order, same "no active cones -> VV body unchanged and cluster geometry
omitted" gating, same output paths/filenames/artifact counts, and the
per-firing export try/except (optional-visualization warn-and-continue).
Optional per-firing meshes are initialized explicitly (no locals() probe).
graph_jfh_thruster_check's fixed demo STL paths are intentionally left
literal (they select dedicated check geometry, not the case STLs).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add 35 new tests; no existing test or fixture is modified. rpod_unit_test_04.py (unit): transform_plume_mesh local/complete placement against hand-computed legacy golden sequences (identity/nontrivial thruster and VV rotations, VV translation, clusters on/off, multi-digit cluster id, exact operation order, not-applied-twice, mutation semantics, unknown thruster / missing cluster KeyError, matrix/vector ValueError); thruster-id mapping (normal, insertion order, reorder, invalid index, caching, cache invalidation on set_thruster_config, one-element name field, canonical output); compose_meshes (zero raises, one returns same object, ordering, None raises, no mutation); and transform_mesh mesh-object API regression. rpod_integration_test_08.py (integration): run graph_jfh and visualize_sweep on base_case and compare produced STL geometry to an inline reproduction of the legacy sequence (atol 1e-3, justified by float32 STL round-trip vs O(0.1)+ transform shifts); assert artifact count/filenames and per-firing face counts; and assert graph_jfh_thruster_check applies the thruster transform exactly once (local mode, no JFH pose). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add docs/pr116_plume_mesh_refactor.md covering ownership of plume placement (VisitingVehicle.transform_plume_mesh), coordinate-frame assumptions, exact transform order, mutation semantics, cluster-id parsing, thruster-index mapping lifecycle, compose_meshes behavior, exception behavior, why PlumeStudyExport stays in pyrpod/rpod, why the old PlumeMeshUtils.py approach was not retained, and deferred follow-ups. Co-Authored-By: Claude Opus 4.8 <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.
No description provided.