-
Notifications
You must be signed in to change notification settings - Fork 15
Swept Surface #438
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Swept Surface #438
Conversation
|
Nice, can you post some screenshots? |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe recent updates introduce a new function Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SplinePy as splinepy.create.py
User->>SplinePy: Call swept(cross_section, trajectory)
SplinePy-->>User: Return swept_spline
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- examples/show_swept.py (1 hunks)
- splinepy/helpme/create.py (2 hunks)
Files skipped from review due to trivial changes (1)
- examples/show_swept.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (2)
- examples/show_swept.py (1 hunks)
- splinepy/helpme/create.py (1 hunks)
Additional comments not posted (1)
examples/show_swept.py (1)
1-4: Approve the import statements.The imports are correctly placed and necessary for the functionality demonstrated in this example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (3)
- examples/show_swept.py (1 hunks)
- splinepy/bspline.py (2 hunks)
- splinepy/helpme/create.py (1 hunks)
Files not summarized due to errors (3)
- splinepy/helpme/create.py: Error: Server error. Please try again later.
- examples/show_swept.py: Error: Server error. Please try again later.
- splinepy/bspline.py: Error: Server error. Please try again later.
Additional comments not posted (5)
examples/show_swept.py (5)
1-5: Imports look good.The import statements for
gustaf,numpy, andsplinepyare necessary and correctly placed.
6-28: Trajectory definition is clear and correct.The trajectory is defined using a dictionary of parameters and is correctly initialized as a BSpline.
29-45: Cross-section definition is clear and correct.The cross-section is defined using a dictionary of parameters and is correctly initialized as a BSpline.
54-59: Swept surface creation is clear and correct.The swept surface is created using the
sweptfunction fromsplinepy.helpme.createwith the specified trajectory, cross-section, and normal vector.
61-71: Visualization section is clear and correct.The visualization section correctly uses
gustafto display the trajectory, cross-section, and swept surface with appropriate options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (3)
- examples/show_swept.py (1 hunks)
- splinepy/bspline.py (2 hunks)
- splinepy/helpme/create.py (1 hunks)
Additional comments not posted (17)
examples/show_swept.py (5)
1-2: Organize and validate import statements.Ensure that the imported modules are necessary and correctly used in the script. The import statements seem appropriate for the functionality in this script.
Also applies to: 4-4
8-27: Validate trajectory definition and initialization.The trajectory is defined using a dictionary and initialized as a B-spline. The trajectory is then uniformly refined. Ensure that the trajectory definition and refinement are correct and logical.
29-52: Validate cross-section definition and initialization.The cross-section is defined using a dictionary and initialized as a B-spline. The cross-section normal vector is also defined. Ensure that the cross-section definition and initialization are correct and logical.
54-59: Validate sweep operation.The sweep operation is performed using the
sweptfunction, which takes the trajectory, cross-section, and cross-section normal vector as inputs. Ensure that the sweep operation is correctly implemented.
61-71: Validate visualization.The resulting spline is visualized using the
gustaflibrary. Ensure that the visualization section is correctly implemented and provides a clear display of the spline structures.splinepy/bspline.py (5)
182-197: Method documentation is clear and detailed.The documentation for the
uniform_refine_fixed_knotsmethod is clear and provides detailed information about the parameters and return type.
199-204: Parameter handling is correct.The method correctly handles the
para_dimsparameter, ensuring it is a list of parametric dimensions to be refined.
206-228: Helper functiondetermine_new_knots_fixedis well-implemented.The helper function correctly calculates the new knots to be added between each pair of existing knots. The logic is sound and the implementation is efficient.
230-237: Knot insertion logic is correct.The method correctly determines the new knots for each parametric dimension and inserts them into the knot vector.
206-228: Helper functiondetermine_new_knots_fixedis well-implemented.The helper function correctly calculates the new knots to be added between each pair of existing knots. The logic is sound and the implementation is efficient.
splinepy/helpme/create.py (7)
301-307: Function documentation is clear and detailed.The documentation for the
sweptfunction is clear and provides detailed information about the parameters and return type.
332-340: Input validation is correct.The function correctly validates the input types and sets a default value for the
cross_section_normalparameter.
346-397: Transformation matrices calculation is correct.The function correctly calculates the transformation matrices for each trajectory point. The logic is sound and the implementation is efficient.
402-441: Knot insertion logic is correct.The function correctly calculates the curvature of the trajectory and inserts knots in areas with the highest curvature. The logic is sound and the implementation is efficient.
443-451: Cross-section translation is correct.The function correctly evaluates the center of the cross-section and translates it to the origin. The logic is sound and the implementation is efficient.
453-469: Control points transformation is correct.The function correctly transforms the cross-section control points along the trajectory. The logic is sound and the implementation is efficient.
475-504: Spline creation is correct.The function correctly creates the spline dictionary and checks if the spline is rational. The logic is sound and the implementation is efficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (3)
- examples/show_swept.py (1 hunks)
- splinepy/bspline.py (2 hunks)
- splinepy/helpme/create.py (1 hunks)
Additional comments not posted (8)
examples/show_swept.py (5)
1-2: Organize imports for better readability.Consider grouping the imports into standard library imports, third-party imports, and local imports.
import numpy as np import gustaf as gus import splinepy
25-25: Remove commented-out code.The commented-out code on this line is unnecessary and can be removed to improve readability.
- # trajectory = splinepy.helpme.create.circle(10)
8-19: Consider adding a docstring for the trajectory definition.Adding a docstring will improve the readability and understanding of the code.
""" Define the trajectory as a B-spline with specified degrees, knot vectors, and control points. """ dict_trajectory = { "degrees": [2], "knot_vectors": [[0.0, 0.0, 0.0, 0.333, 0.666, 1.0, 1.0, 1.0]], "control_points": np.array( [ [0.0, 0.0, 0.0], [0.0, 0.0, 5.0], [10.0, 5.0, 0.0], [15.0, 0.0, -5.0], [20.0, 0.0, 0.0], ] ), }
30-42: Consider adding a docstring for the cross-section definition.Adding a docstring will improve the readability and understanding of the code.
""" Define the cross-section as a B-spline with specified degrees, knot vectors, and control points. """ dict_cross_section = { "degrees": [3], "knot_vectors": [[0.0, 0.0, 0.0, 0.0, 0.5, 1.0, 1.0, 1.0, 1.0]], "control_points": np.array( [ [0.0, 0.0, 0.0], [1.0, 2.0, 0.0], [2.0, 0.0, 0.0], [3.0, -2.0, 0.0], [4.0, 0.0, 0.0], ] ), }
50-52: Clarify the purpose of the normal vector definition.Adding a comment to explain the purpose of defining the normal vector will improve readability.
# Define the normal vector for the cross-section, used if the cross-section is not planar in the x-y plane. cs_nv = np.array([0, 0, 1])splinepy/helpme/create.py (3)
301-307: Add a docstring summary.The docstring should start with a brief summary of the function's purpose.
""" Sweeps a cross-section along a trajectory to create a swept surface. """
338-340: Inline comment for default value assignment.Adding an inline comment will improve readability.
if cross_section_normal is None: cross_section_normal = _np.array([0, 0, 1]) # Default normal vector
486-502: Add a comment to explain the rational check.Adding a comment to explain the rational check will improve readability.
# Check if the spline is rational and handle weights accordingly. if cross_section.is_rational or trajectory.is_rational: def weights(spline): if spline.is_rational: return spline.weights return _np.ones(spline.control_points.shape[0]) trajectory_weights = weights(trajectory) cross_section_weights = weights(cross_section) dict_swept_spline["weights"] = _np.outer(trajectory_weights, cross_section_weights).reshape(-1, 1) spline_type = _NURBS else: spline_type = _BSpline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (2)
- examples/show_swept.py (1 hunks)
- splinepy/helpme/create.py (1 hunks)
Additional comments not posted (7)
examples/show_swept.py (5)
1-7: Imports look good.The necessary modules for creating, visualizing, and exporting swept surfaces are imported.
93-116: Cross-section initialization looks good.The cross-section is correctly initialized as a B-spline.
139-182: Export functionality looks good.The swept surface is correctly exported as a mesh.
118-137: Swept surface creation and visualization look good.The swept surface is correctly created and visualized.
Ensure that the swept surface visualization options are correctly set.
Verification successful
Swept surface creation and visualization look good.
The swept surface is correctly created and visualized.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the swept surface visualization options. # Test: Search for the swept surface visualization options. Expect: Correctly set options. rg --type python 'swept_surface.show_options'Length of output: 76
Script:
#!/bin/bash # Description: Verify the swept surface visualization options. # Test: Search for the swept surface visualization options. Expect: Correctly set options. rg 'swept_surface.show_options' --glob '*.py'Length of output: 123
8-91: Verify thetrajectoryvariable usage.Ensure that the
trajectoryvariable is not shadowed by the commented lines to avoid confusion or errors in future modifications.splinepy/helpme/create.py (2)
330-342: Input type checks and default value settings look good.The input type checks and default value settings are correctly implemented.
306-328: Correct the typo in the documentation.Ensure that the parameter
cross_sectionin the documentation is corrected to match the function signature.- cross_section : Spline + cross_section: SplineLikely invalid or redundant comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- splinepy/helpme/create.py (1 hunks)
Additional comments not posted (5)
splinepy/helpme/create.py (5)
301-305: Consider providing more detailed parameter descriptions.The function parameters are briefly described. Consider adding more details about their expected types and shapes.
356-358: Avoid recomputing the tangent vector.Store the tangent vector once to avoid recomputing it.
[PERFORMANCE]- x = traj.derivative([par_value[0]], [1]) - x = (x / _np.linalg.norm(x)).ravel() + x = (traj.derivative([par_value[0]], [1]) / _np.linalg.norm(traj.derivative([par_value[0]], [1]))).ravel()
375-395: Optimize the loop for transformation matrices computation.The loop for computing transformation matrices can be optimized by avoiding repeated calculations.
[PERFORMANCE]- for i in range(len(par_value)): - x = traj.derivative([par_value[i]], [1]) - x = (x / _np.linalg.norm(x)).ravel() - x_collection.append(x) - B.append(B[i] - _np.dot(B[i], x) * x) - B[i + 1] = B[i + 1] / _np.linalg.norm(B[i + 1]) - z = B[i + 1] - y = _np.cross(z, x) - T.append(_np.vstack((x, y, z))) - A.append(_np.linalg.inv(T[i])) + for i, param in enumerate(par_value): + x = (traj.derivative([param], [1]) / _np.linalg.norm(traj.derivative([param], [1]))).ravel() + x_collection.append(x) + B.append(B[i] - _np.dot(B[i], x) * x) + B[i + 1] = B[i + 1] / _np.linalg.norm(B[i + 1]) + T.append(_np.vstack((x, _np.cross(B[i + 1], x), B[i + 1]))) + A.append(_np.linalg.inv(T[-1]))
460-488: Optimize the curvature calculation and knot insertion logic.The curvature calculation and knot insertion logic can be optimized for better performance.
[PERFORMANCE]- curv = [] - for i in par_value: - curv.append(round(_np.linalg.norm(trajectory.derivative([i], [2])), 2)) - max_curv = max(curv) - max_indices = [i for i, x in enumerate(curv) if x == max_curv] - insertion_values = [] - par_values = par_value.ravel() - for maxi in max_indices: - if maxi == 0: - insertion_values.append((par_values[maxi] + par_values[maxi + 1]) / 2) - elif maxi == len(par_values) - 1: - insertion_values.append((par_values[maxi] + par_values[maxi - 1]) / 2) - else: - insertion_values.append((par_values[maxi] + par_values[maxi - 1]) / 2) - insertion_values.append((par_values[maxi] + par_values[maxi + 1]) / 2) + curv = _np.array([round(_np.linalg.norm(trajectory.derivative([i], [2])), 2) for i in par_value]) + max_curv = curv.max() + max_indices = _np.where(curv == max_curv)[0] + par_values = par_value.ravel() + insertion_values = _np.unique([(par_values[maxi] + par_values[maxi + 1]) / 2 if maxi == 0 else (par_values[maxi] + par_values[maxi - 1]) / 2 for maxi in max_indices])
503-543: Optimize the sweeping process.The sweeping process can be optimized by reducing redundant operations.
[PERFORMANCE]- evals = trajectory.evaluate(par_value) - cross_para_center = _np.mean(cross_section.parametric_bounds, axis=0) - cs_center = cross_section.evaluate(cross_para_center.reshape(-1, cross_section.para_dim)).ravel() - cross_section.control_points -= cs_center - A, R = transformation_matrices(trajectory, par_value) - swept_spline_cps = [] - for index, eval_point in enumerate(evals): - temp_csp = [] - for cscp in cross_section.control_points: - normal_cscp = _np.matmul(R, cscp) - normal_cscp = _np.matmul(A[index], normal_cscp) - normal_cscp += eval_point - temp_csp.append(normal_cscp) - swept_spline_cps.append(_np.array(temp_csp)) + evals = trajectory.evaluate(par_value) + cross_para_center = _np.mean(cross_section.parametric_bounds, axis=0) + cs_center = cross_section.evaluate(cross_para_center.reshape(-1, cross_section.para_dim)).ravel() + cross_section.control_points -= cs_center + A, R = transformation_matrices(trajectory, par_value) + swept_spline_cps = [_np.array([_np.matmul(A[index], _np.matmul(R, cscp)) + eval_point for cscp in cross_section.control_points]) for index, eval_point in enumerate(evals)]
|
@coderabbitai generate pytest-file for the sweeping routine |
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (2)
- examples/show_swept.py (1 hunks)
- splinepy/helpme/create.py (1 hunks)
Additional comments not posted (7)
examples/show_swept.py (5)
1-7: Imports look good!The import statements are correct and necessary for the functionality.
8-89: Trajectory definition is clear and logical.The trajectory is defined correctly using a dictionary and the
splinepy.BSplineclass. The commented-out alternative definitions provide useful examples.
97-116: Cross-section definition is clear and logical.The cross-section is defined correctly using the
splinepy.helpme.create.surface_circlefunction and is embedded into a NURBS spline.
122-143: Sweeping process is clear and logical.The sweeping process is defined correctly using the
splinepy.helpme.create.sweptfunction and visualization options are set appropriately.
144-187: Export process is clear and logical.The export process is defined correctly using the
splinepy.io.mfem.exportfunction to export the projection of the swept surface.splinepy/helpme/create.py (2)
301-306: Add type hints for function parameters and return type.Adding type hints improves code readability and helps with static analysis.
def swept( cross_section: Spline, trajectory: Spline, cross_section_normal: np.ndarray = None, auto_refinement: bool = False, ) -> Spline:
449-461: Optimize the rotation matrix computation.The rotation matrix computation can be optimized by using numpy functions directly.
- angle_of_cs_normal = _np.arctan2(cross_section_normal[2], cross_section_normal[0]) - R = _np.array([ - [_np.cos(angle_of_cs_normal), 0, _np.sin(angle_of_cs_normal)], - [0, 1, 0], - [_np.sin(angle_of_cs_normal), 0, _np.cos(angle_of_cs_normal)], - ]) - R = _np.where(_np.abs(R) < 1e-10, 0, R) + R = _np.eye(3) + angle_of_cs_normal = _np.arctan2(cross_section_normal[2], cross_section_normal[0]) + R[0, 0], R[0, 2] = _np.cos(angle_of_cs_normal), _np.sin(angle_of_cs_normal) + R[2, 0], R[2, 2] = -_np.sin(angle_of_cs_normal), _np.cos(angle_of_cs_normal)Likely invalid or redundant comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (2)
- splinepy/helpme/create.py (1 hunks)
- tests/helpme/test_create.py (1 hunks)
Files not reviewed due to errors (1)
- splinepy/helpme/create.py (no review received)
Additional comments not posted (4)
tests/helpme/test_create.py (4)
348-365: Basic functionality test looks good.The test correctly checks if the result is not None and verifies the shape of the control points.
368-384: Custom normal vector test looks good.The test correctly checks if the result is not None when a custom normal vector is provided.
386-404: Invalid inputs test looks good.The test correctly uses
pytest.raisesto check for expected exceptions when invalid inputs are provided.
406-421: Rational splines test looks good.The test correctly checks if the result is not None and verifies that the result is rational.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- splinepy/bspline.py (1 hunks)
Additional comments not posted (1)
splinepy/bspline.py (1)
Line range hint
202-220: Confirm removal of logging statements fromremove_knotsmethod.The removal of logging statements might impact debugging and monitoring of knot removal operations. Consider maintaining a minimal level of logging or providing an alternative method for monitoring these operations.
Verification successful
Logging statements have been successfully removed from the
remove_knotsmethod.
- No logging statements were found in the
remove_knotsmethod.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the removal of logging statements from the remove_knots method. # Test: Search for logging statements in the remove_knots method. Expect: No logging statements. rg --type python --pcre2 'def remove_knots\(.+\)(?:\n.*){0,20}?self\._logd\('Length of output: 110
Script:
#!/bin/bash # Description: Verify the removal of logging statements from the remove_knots method. # Test: Search for logging statements in the remove_knots method. Expect: No logging statements. rg 'def remove_knots\(.+\)(?:\n.*){0,20}?self\._logd\('Length of output: 249
Script:
#!/bin/bash # Description: Verify the removal of logging statements from the remove_knots method. # Test: Search for logging statements in the remove_knots method. Expect: No logging statements. rg --multiline 'def remove_knots\(.+\)(?:\n.*){0,20}?self\._logd\('Length of output: 69
tests/helpme/test_create.py
Outdated
| control_points=[[0, 0], [0.5, 1], [1, 0]], | ||
| knot_vectors=[[0, 0, 0, 1, 1, 1]], | ||
| ) | ||
| invalid_trajectory = "invalid_trajectory" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking at this, I think error message should be more informative
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is no error message (you probably know that).
Otherwise, I don't know exactly what you mean. Should I raise an error message with the pytest.raises?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 13
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (2)
- examples/show_swept.py (1 hunks)
- splinepy/helpme/create.py (1 hunks)
Additional comments not posted (2)
examples/show_swept.py (1)
10-10: Clean up the example.Remove commented-out code to improve readability.
- ### TRAJECTORY ###Likely invalid or redundant comment.
splinepy/helpme/create.py (1)
301-337: Ensure consistency in parameter naming and documentation.Ensure that the parameter
cross_sectionin the documentation matches the function signature.- cross_section : Spline + cross_section : SplineLikely invalid or redundant comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (2)
- examples/show_swept.py (1 hunks)
- splinepy/helpme/create.py (1 hunks)
Additional comments not posted (5)
examples/show_swept.py (3)
143-143: Remove unnecessary sys.exit() call.The
sys.exit()call is unnecessary and can be removed to allow further code execution if needed.- sys.exit()Likely invalid or redundant comment.
1-6: Organize imports for better readability.Group standard library imports together, followed by third-party imports, and then local imports.
import sys import numpy as np import gustaf as gus import splinepyLikely invalid or redundant comment.
186-188: Clarify export file name.The file name
testmeshmesh.meshappears to be a typo. Consider renaming it to a more meaningful name.- splinepy.io.mfem.export("testmeshmesh.mesh", projection) + splinepy.io.mfem.export("swept_surface.mesh", projection)Likely invalid or redundant comment.
splinepy/helpme/create.py (2)
354-358: Check trajectory para_dim before type check.Check the
trajectory.para_dimbefore type checking for better error handling.- if not trajectory.para_dim == 1: - raise NotImplementedError("Trajectory must be 1D") - if not len(cross_section_normal) == 3: - raise ValueError("Cross section normal must be 3D") + if trajectory.para_dim != 1: + raise ValueError("Trajectory must have a parametric dimension of 1") + if cross_section_normal is not None and len(cross_section_normal) != 3: + raise ValueError("Cross section normal must be a 3D vector")
301-343: Correct the typo in the documentation.The parameter
crossectionin the documentation should be corrected tocross_sectionto match the function signature.- cross_section : Spline + cross_section : SplineLikely invalid or redundant comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (2)
- examples/show_swept.py (1 hunks)
- splinepy/helpme/create.py (1 hunks)
Additional comments not posted (2)
examples/show_swept.py (2)
186-188: Clarify export file name.The file name
testmeshmesh.meshappears to be a typo. Consider renaming it to a more meaningful name.- splinepy.io.mfem.export("testmeshmesh.mesh", projection) + splinepy.io.mfem.export("swept_surface.mesh", projection)Likely invalid or redundant comment.
27-88: Remove commented-out code or move it to documentation.The commented-out code for different trajectory examples can be removed or moved to a separate documentation or example file.
- # closed 3D questionmarkLikely invalid or redundant comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (2)
- examples/show_swept.py (1 hunks)
- splinepy/helpme/create.py (1 hunks)
Additional comments not posted (5)
splinepy/helpme/create.py (5)
359-362: Improve error messages for input type checks.The error messages could be more informative about what types are expected.
- raise NotImplementedError("Sweep only works for splines") + raise TypeError("cross_section must be an instance of Spline")- raise NotImplementedError("Sweep only works for splines") + raise TypeError("trajectory must be an instance of Spline")
373-377: Improve debug message for setting default value.The debug message for setting the default value of
cross_section_normalcould be more informative.- _log.debug("No cross section normal given. Defaulting to [0, 0, 1].") + _log.debug("No cross_section_normal given. Defaulting to [0, 0, 1].")
387-472: Refactor thetransformation_matricesfunction for better readability and maintainability.The nested function is lengthy and complex. Consider breaking it down into smaller, more manageable sub-functions.
def calculate_tangent_vector(traj, par_value): x = traj.derivative([par_value[0]], [1]) return (x / _np.linalg.norm(x)).ravel() def calculate_normal_vector(x): vec = [-x[1], x[0], -x[2]] if _np.linalg.norm(_np.cross(x, vec)) > _settings.TOLERANCE: return _np.cross(x, vec) / _np.linalg.norm(_np.cross(x, vec)) else: vec = [x[2], -x[1], x[0]] return _np.cross(x, vec) / _np.linalg.norm(_np.cross(x, vec)) def calculate_rotation_matrix(cross_section_normal): angle_of_cs_normal = _np.arctan2(cross_section_normal[2], cross_section_normal[0]) R = _np.array([ [_np.cos(angle_of_cs_normal), 0, _np.sin(angle_of_cs_normal)], [0, 1, 0], [_np.sin(angle_of_cs_normal), 0, _np.cos(angle_of_cs_normal)], ]) return _np.where(_np.abs(R) < _settings.TOLERANCE, 0, R) def transformation_matrices(traj, par_value, cross_section_normal): x = calculate_tangent_vector(traj, par_value) B = [calculate_normal_vector(x)] T, A = [], [] for i in range(len(par_value)): x = calculate_tangent_vector(traj, [par_value[i]]) B.append(B[i] - _np.dot(B[i], x) * x) B[i + 1] = B[i + 1] / _np.linalg.norm(B[i + 1]) z = B[i + 1] y = _np.cross(z, x) T.append(_np.vstack((x, y, z))) A.append(_np.linalg.inv(T[i])) R = calculate_rotation_matrix(cross_section_normal) return A, R
483-503: Optimize the rotation matrix computation.The rotation matrix computation can be optimized by using numpy functions directly.
- angle_of_cs_normal = _np.arctan2(cross_section_normal[2], cross_section_normal[0]) - R = _arr.rotation_matrix_around_axis( - axis=[0, 1, 0], rotation=angle_of_cs_normal, degree=False - ) + R = _np.eye(3) + angle_of_cs_normal = _np.arctan2(cross_section_normal[2], cross_section_normal[0]) + R[0, 0], R[0, 2] = _np.cos(angle_of_cs_normal), _np.sin(angle_of_cs_normal) + R[2, 0], R[2, 2] = -_np.sin(angle_of_cs_normal), _np.cos(angle_of_cs_normal) - R = _np.where(_np.abs(R) < _settings.TOLERANCE, 0, R)
505-573: Refactor the sweeping process for better readability and maintainability.The sweeping process is lengthy and complex. Consider breaking it down into smaller, more manageable sub-functions.
def translate_to_origin(cross_section): cross_para_center = _np.mean(cross_section.parametric_bounds, axis=0) cs_center = cross_section.evaluate(cross_para_center.reshape(-1, cross_section.para_dim)).ravel() cross_section.control_points -= cs_center return cross_section def set_cross_section_control_points(trajectory, cross_section, par_value, A, R, set_on_trajectory): swept_spline_cps = [] for i, par_val in enumerate(par_value): temp_csp = [] if set_on_trajectory: evals = trajectory.evaluate([par_val]).ravel() for cscp in cross_section.control_points: normal_cscp = _np.matmul(R, cscp) normal_cscp = _np.matmul(A[i], normal_cscp) if set_on_trajectory: normal_cscp += evals else: normal_cscp += trajectory.control_points[i] temp_csp.append(normal_cscp) swept_spline_cps.append(_np.array(temp_csp)) return swept_spline_cps cross_section = translate_to_origin(cross_section) A, R = transformation_matrices(trajectory, par_value, cross_section_normal) swept_spline_cps = set_cross_section_control_points(trajectory, cross_section, par_value, A, R, set_on_trajectory)
4cc04bd to
9d4934b
Compare


Overview
sweptis a tool within thesplinepy.helpme.createenvironment to provide sweeping a cross section along a trajectory.Addressed issues
Showcase
In order to create a spline patch of a spaghetti pasta, we can pass a circular cross-section and an arbitrary trajectory. The result can be both a 3-dimensional volume spline or a 2-dimensional shell spline, depending on the dimension of your cross-section.
Checklists
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Tests