Skip to content

Conversation

@OberGue
Copy link
Member

@OberGue OberGue commented Jun 12, 2024

Overview

swept is a tool within the splinepy.helpme.create environment to provide sweeping a cross section along a trajectory.

Addressed issues

  • Referring to this discussion topic.

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.

import splinepy

dict_trajectory = {
        "degrees": [3],
        "knot_vectors": [[0.0, 0.0, 0.0, 0.0, 0.2, 0.4, 0.6, 0.8, 0.9, 1.0, 1.0, 1.0, 1.0]],
        "control_points": np.array(
            [   [0.5, 0],
                [0.5, 2],
                [1.0, 3],
                [2.0, 4],
                [2.15, 5],
                [1.8, 5.9],
                [1.0, 6.2],
                [-0.25, 6],
                [-0.5, 5],]),}

trajectory = splinepy.BSpline(**dict_trajectory)
cross_section = splinepy.helpme.create.surface_circle(0.5).nurbs

spaghetti = splinepy.helpme.create.swept(cross_section, trajectory)

Checklists

  • Documentations are up-to-date.
  • Added example(s)
  • Added test(s)

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced a new function for sweeping a cross-section along a specified trajectory, enhancing the creation of complex spline shapes.
    • Improved input validation for cross-section and trajectory parameters to ensure compatibility with spline types.
  • Tests

    • Added new test functions to evaluate the sweeping functionality, including basic operations, comparison with extruded surfaces, and validation of control point placement and alignment with the trajectory.

@jzwar
Copy link
Collaborator

jzwar commented Jun 12, 2024

Nice, can you post some screenshots?

@OberGue
Copy link
Member Author

OberGue commented Jun 12, 2024

Nice, can you post some screenshots?

Sure! Be aware that work is still in progress.

2D_S_topfront_view
3D_questionmark_topfrontview

@coderabbitai
Copy link

coderabbitai bot commented Jun 20, 2024

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The recent updates introduce a new function swept in the splinepy/helpme/create.py file, enabling the sweeping of a cross-section along a specified trajectory. This function includes input validation for the cross_section and trajectory, ensures the trajectory is one-dimensional, and applies transformations to create a new swept spline. Additionally, several new test functions are added in tests/helpme/test_create.py to validate the functionality of the swept method, enhancing test coverage without altering existing functions.

Changes

Files Change Summaries
splinepy/helpme/create.py Added the swept function for sweeping a cross-section along a trajectory with input validation and transformation logic.
tests/helpme/test_create.py Added three new test functions: test_swept_basic_functionality, test_swept_to_extruded, and test_swept_control_point_placing to validate the swept method's functionality.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant SplinePy as splinepy.create.py

    User->>SplinePy: Call swept(cross_section, trajectory)
    SplinePy-->>User: Return swept_spline
Loading

Poem

In the realm of splines, a journey begins,
Sweeping cross-sections, where magic spins.
Along paths they glide, with elegance and grace,
Creating new forms in a mathematical space.
With each twist and turn, new shapes come to be,
A dance of dimensions, wild and free.
🌈✨

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ft-swept

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@j042 j042 marked this pull request as ready for review June 21, 2024 10:46
Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 6d3e137 and 48b4bbb.

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

@tataratat tataratat deleted a comment from j042 Jun 27, 2024
Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 48b4bbb and 0c90044.

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.

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 0c90044 and 4387404.

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, and splinepy are 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 swept function from splinepy.helpme.create with the specified trajectory, cross-section, and normal vector.


61-71: Visualization section is clear and correct.

The visualization section correctly uses gustaf to display the trajectory, cross-section, and swept surface with appropriate options.

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 0c90044 and 4387404.

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 swept function, 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 gustaf library. 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_knots method is clear and provides detailed information about the parameters and return type.


199-204: Parameter handling is correct.

The method correctly handles the para_dims parameter, ensuring it is a list of parametric dimensions to be refined.


206-228: Helper function determine_new_knots_fixed is 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 function determine_new_knots_fixed is 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 swept function 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_normal parameter.


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.

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 0c90044 and 4387404.

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

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 4387404 and 6737fb8.

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 the trajectory variable usage.

Ensure that the trajectory variable 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_section in the documentation is corrected to match the function signature.

-    cross_section : Spline
+    cross_section: Spline

Likely invalid or redundant comment.

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 6737fb8 and cc32216.

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)]

@tataratat tataratat deleted a comment from coderabbitai bot Jul 2, 2024
@OberGue
Copy link
Member Author

OberGue commented Jul 2, 2024

@coderabbitai generate pytest-file for the sweeping routine

@coderabbitai

This comment was marked as outdated.

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between cc32216 and 045a493.

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.BSpline class. 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_circle function 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.swept function 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.export function 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.

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 045a493 and 3262829.

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.raises to 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.

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 3262829 and adf3ae7.

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 from remove_knots method.

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_knots method.

  • No logging statements were found in the remove_knots method.
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

control_points=[[0, 0], [0.5, 1], [1, 0]],
knot_vectors=[[0, 0, 0, 1, 1, 1]],
)
invalid_trajectory = "invalid_trajectory"
Copy link
Member

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

Copy link
Member Author

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between adf3ae7 and 8790d3d.

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_section in the documentation matches the function signature.

- cross_section : Spline
+ cross_section : Spline

Likely invalid or redundant comment.

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 8790d3d and ca27460.

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 splinepy

Likely invalid or redundant comment.


186-188: Clarify export file name.

The file name testmeshmesh.mesh appears 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_dim before 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 crossection in the documentation should be corrected to cross_section to match the function signature.

- cross_section : Spline
+ cross_section : Spline

Likely invalid or redundant comment.

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between ca27460 and 73a3839.

Files selected for processing (1)
  • splinepy/helpme/create.py (1 hunks)

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 73a3839 and ca5adbf.

Files selected for processing (1)
  • splinepy/helpme/create.py (1 hunks)

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between ca5adbf and 1855ad4.

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.mesh appears 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 questionmark

Likely invalid or redundant comment.

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 1855ad4 and 87c5193.

Files selected for processing (2)
  • splinepy/helpme/create.py (1 hunks)
  • tests/helpme/test_create.py (1 hunks)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

Commits

Files that changed from the base of the PR and between 87c5193 and bfcebfa.

Files selected for processing (1)
  • tests/helpme/test_create.py (1 hunks)

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between bfcebfa and 40a6d8e.

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_normal could 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 the transformation_matrices function 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)

OberGue added 28 commits August 18, 2025 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants