splinepy icon indicating copy to clipboard operation
splinepy copied to clipboard

Test microstructure and -tiles

Open markriegler opened this issue 1 year ago • 9 comments

Overview

Addressing issue #392: this should add tests related to all microstructure- and microtiles-related code.

Addressing issue #458: some changes to the code of the microtiles and possibly the microstructure are needed in order to satisfy the tests.

Additionally, the code's layout for the tiles is unified, in particular:

  • Add class variables (e.g. are sensitivities implemented, the bounds for the parameters, implemented closure directions, default parameter values)
  • Add unified function in microstructure to check tile's input and custom parameters
  • Unify order of actions within tiles' create_tile() and _closing_tile()

Checklist of what to test

  • [x] Derivatives of the tiles (incl. tiles with closure)
  • [x] Bound checks of tiles (incl. tiles with closure)
  • [ ] ...

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced support for sensitivity analysis and parameter derivatives across multiple tile classes.
    • Added new attributes for parameter bounds, shapes, default values, and closure directions in many tile classes.
    • Enhanced parameter handling, validation, and processing with unified internal methods.
    • Added closure direction support and refined tile creation methods in HollowOctagonExtrude, HollowOctagon, Cross2D, Cross3D, and Cross3DLinear.
    • Updated create_tile and closing tile methods to support parameter sensitivities and derivative computations.
    • Extended test coverage with comprehensive tests validating tile construction, parameter handling, closure behavior, and derivative accuracy.
    • Fixed consistent typo in example scripts for keyword argument spelling.
  • Bug Fixes

    • Corrected typographical errors in documentation and parameter names.
    • Fixed rotation matrix derivative definitions in some tile classes.
    • Improved error messages and validation feedback.
  • Tests

    • Added new fixtures for reproducible random number generation and derivative testing parameters.
    • Implemented extensive parameterized tests for microstructure tiles covering closure, parameter bounds, derivatives, and invalid inputs.
    • Enhanced tests for microstructure closure faces and macro sensitivity correctness.

markriegler avatar Aug 29 '24 09:08 markriegler

Walkthrough

The changes update multiple files in the splinepy library by fixing typos, adding new class attributes for sensitivity analysis and parameter bounds, refactoring parameter validation and input processing into unified methods, and enhancing tile classes with closure and derivative support. Additionally, comprehensive unit tests were added for tile classes and microstructure functionality to verify correctness and robustness.

Changes

Files Change Summary
splinepy/microstructure/microstructure.py Fixed spelling error in docstrings; updated create method defaults for knot_span_wise and macro_sensitivities; added boolean assertions for parameters.
splinepy/microstructure/tiles/chi.py Changed _evaluation_points from 2D to 1D array; added _sensitivities_implemented, _parameter_bounds, _parameters_shape, _default_parameter_value; refactored create_tile to use _process_input for parameter and sensitivity handling; adjusted derivative computations.
splinepy/microstructure/tiles/cube_void.py Added sensitivity-related class attributes; fixed derivative matrix row in _rotation_matrix_y_deriv; refactored create_tile to use _process_input for parameters and sensitivities.
splinepy/microstructure/tiles/ellips_v_oid.py Added sensitivity-related class attributes; fixed derivative matrix row in _rotation_matrix_y_deriv; refactored create_tile to use _process_input for parameter and sensitivity handling; added TODO comment about test skipping.
splinepy/microstructure/tiles/hollow_octagon_extrude.py Added sensitivity, closure, parameter bounds attributes; updated create_tile to accept closure parameter; refactored parameter validation and sensitivity handling using _process_input; replaced closing_tile with _closing_tile method supporting closure directions and derivatives.
splinepy/microstructure/tiles/snappy.py Added sensitivity and closure-related attributes; relaxed type checks for parameters; replaced inline parameter validation with _check_custom_parameter; updated _closing_tile to raise NotImplementedError for unsupported closures and removed parameter checks.
splinepy/microstructure/tiles/tile_base.py Added new class attributes for sensitivity, closure, parameter bounds, shapes, and defaults; converted classmethods to properties; enhanced check_params to validate parameter bounds; added _process_input and _check_custom_parameter methods for unified parameter and sensitivity processing and validation.
splinepy/microstructure/tiles/cross_2d.py Added sensitivity, closure, parameter bounds, and default attributes; refactored _closing_tile and create_tile to use _process_input and _check_custom_parameter for parameter validation and processing; removed manual parameter checks.
splinepy/microstructure/tiles/cross_3d.py Added sensitivity, closure, parameter bounds, and default attributes; refactored _closing_tile and create_tile to unify parameter processing with _process_input and validate parameters with _check_custom_parameter; added closure direction validation and error raising.
splinepy/microstructure/tiles/cross_3d_linear.py Added sensitivity, closure, parameter bounds, and default attributes; refactored _closing_tile and create_tile to use _process_input and _check_custom_parameter for parameter handling; removed manual parameter validity checks.
splinepy/microstructure/tiles/double_lattice.py Added sensitivity and parameter bounds attributes; refactored create_tile to use _process_input and _check_custom_parameter for parameter and contact length validation; removed manual parameter checks.
splinepy/microstructure/tiles/hollow_cube.py Added sensitivity and parameter bounds attributes; refactored create_tile to use _process_input for parameter and sensitivity handling; removed explicit parameter validation logic.
splinepy/microstructure/tiles/hollow_octagon.py Added sensitivity, closure, parameter bounds, and default attributes; added create_tile method; refactored _closing_tile to support parameter sensitivities and derivatives with loops; unified parameter processing with _process_input; added contact length validation; replaced previous hardcoded control point logic with derivative-aware approach.
splinepy/microstructure/tiles/__init__.py Added cross_3d to imports and __all__ export list.
examples/show_microstructures.py Fixed typo in keyword argument name from "seperator_distance" to "separator_distance" in calls to generator.create() and generator.show().
tests/conftest.py Modified np_rng fixture to use fixed seed for reproducibility; added fixtures h_eps, n_test_points, big_perturbation, and fd_derivative_stepsizes_and_weights for numerical derivative testing.
tests/test_knot_vectors.py Updated test assertion to use .numpy() method instead of np.array() for knot vector conversion.
tests/test_microstructure.py Added tests for microstructure tiles covering closure face correctness and macro sensitivity derivative verification using finite differences.
tests/test_microtiles.py Added comprehensive tests for tile classes including control point bounds, parameter validation, closure support, and derivative correctness using finite difference approximations; included tests for invalid parameter values.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant TileBase
    participant TileClass
    participant ParameterProcessor

    User->>TileClass: create_tile(parameters, parameter_sensitivities)
    TileClass->>TileBase: _process_input(parameters, parameter_sensitivities)
    TileBase-->>TileClass: processed_parameters, n_derivatives, derivatives
    TileClass->>TileClass: construct_splines_with_derivatives()
    TileClass-->>User: splines, derivatives
sequenceDiagram
    participant User
    participant TileClass
    participant TileBase

    User->>TileClass: create_tile(closure=direction)
    TileClass->>TileBase: _process_input(parameters, parameter_sensitivities)
    TileBase-->>TileClass: processed_parameters, n_derivatives, derivatives
    TileClass->>TileClass: _closing_tile(closure=direction)
    TileClass-->>User: closing_splines, derivatives

🐇 I hop through code with joyful cheer,
Fixing typos, making defaults clear.
Parameters checked with bounds so tight,
Sensitivities handled just right.
Tiles now dance with derivatives too,
Tests ensure the code is true!
Hoppity-hop, improvements anew! 🐇✨

✨ Finishing Touches
  • [ ] 📝 Generate Docstrings

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.
    • Explain this complex logic.
    • 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. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • 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 src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

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

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

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

Documentation and Community

  • 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.

coderabbitai[bot] avatar Aug 29 '24 09:08 coderabbitai[bot]

The current status is:

  • [x] Added test to check if tile classes have the correct variables and functions
  • [x] Added new variables (e.g. if sensitivities are implemented for this class)
  • [x] Added bound test: check if all control points of tile (with and without closure) lie in unit square/cube
    • Right now, two classes are skipped for testing:
      • EllipsVoid (because the control points easily lie outside of unit cube, but the tile itself lies in the unit cube)
      • Snappy (because there is no parameters-array as an input, but additional parameters like a, b, r and the test is constructed for the parameters-array as an input)
  • [x] Add test for tile derivatives: test derivatives of tiles w.r.t. the parameters by comparing implemented derivatives to the ones obtained by finite differences. This also checks if derivatives work when closure is applied
    • Right now, some classes are skipped due to wrongly implemented derivatives, see issue #458

markriegler avatar Aug 29 '24 09:08 markriegler

I just checked the tests on my local laptop. My implemented tests are flaky due to the random points: most of the time, all tests pass, but sometimes there is one failing test. I think sticking to a fixed random seed might be a better idea, for replicability

markriegler avatar Jan 15 '25 10:01 markriegler

The docs/minimal_explicit_build_and_docs build is not working. @clemens-fricke Any idea, how to solve it? It says Could not find compiler set in environment variable CC: clang-14.

markriegler avatar Feb 07 '25 11:02 markriegler

The docs/minimal_explicit_build_and_docs build is not working. @clemens-fricke Any idea, how to solve it? It says Could not find compiler set in environment variable CC: clang-14.

I will look into it when I am back from holiday. But I know that @j042 was already looking into it

clemens-fricke avatar Feb 07 '25 11:02 clemens-fricke

@markriegler is this complete?

j042 avatar May 30 '25 13:05 j042

@markriegler is this complete?

The tests I wanted to implement are complete. Also the code is now more unified, which makes it easier for future implementations.

markriegler avatar Jun 02 '25 13:06 markriegler

@markriegler is this complete?

The tests I wanted to implement are complete. Also the code is now more unified, which makes it easier for future implementations.

Ok, is that yes?

j042 avatar Jun 07 '25 08:06 j042

Yes

markriegler avatar Jun 07 '25 08:06 markriegler