Skip to content
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

feat: add augurs-dtw crate with dynamic time warping implementation #98

Merged
merged 26 commits into from
Aug 21, 2024
Merged

Conversation

sd2k
Copy link
Collaborator

@sd2k sd2k commented Jul 15, 2024

Adds a new crate, augurs-dtw, with an implementation of dynamic time warping. This uses the same implementation as the UCR Suite.

TODO

Summary by CodeRabbit

  • New Features

    • Introduced the augurs-dtw package for Dynamic Time Warping (DTW) functionality.
    • Added support for distance calculations using Euclidean and Manhattan metrics.
    • Enhanced distance matrix computations with parallel processing options.
    • New Dtw class for Python, allowing easy integration and usage of DTW.
    • Implemented a DistanceMatrix class for improved distance matrix handling and interoperability.
  • Documentation

    • Updated README files to include details about the augurs-dtw package and its functionalities.
    • Added usage examples for DTW in pyaugurs documentation to improve user guidance.
    • Enhanced clarity regarding the augurs-dtw module's alpha status and flexibility in the README.
  • Chores

    • Added changelogs to document notable changes and adherence to Semantic Versioning.

Copy link
Contributor

github-actions bot commented Jul 15, 2024

🐰Bencher

ReportWed, August 21, 2024 at 16:00:15 UTC
Projectaugurs
Branch98/merge
Testbedubuntu-latest

🚨 1 ALERT: Threshold Boundary Limit exceeded!
BenchmarkMeasure (units)ViewValueLower BoundaryUpper Boundary
distance_euclidean/NoneLatency (nanoseconds (ns))🚨 (view plot | view alert)210,590.00 (+3.91%)205,401.69 (102.53%)

Click to view all benchmark results
BenchmarkLatencyLatency Results
nanoseconds (ns) | (Δ%)
Latency Upper Boundary
nanoseconds (ns) | (%)
auto_fit/air_passengers✅ (view plot)1,902,100.00 (-1.41%)1,994,715.54 (95.36%)
distance_euclidean/None🚨 (view plot | view alert)210,590.00 (+3.91%)205,401.69 (102.53%)
distance_euclidean/Some(10)✅ (view plot)15,615.00 (-0.44%)15,764.74 (99.05%)
distance_euclidean/Some(2)✅ (view plot)3,555.40 (+0.99%)3,587.82 (99.10%)
distance_euclidean/Some(20)✅ (view plot)31,671.00 (+0.39%)31,906.56 (99.26%)
distance_euclidean/Some(5)✅ (view plot)7,787.00 (-1.41%)8,212.98 (94.81%)
distance_euclidean/Some(50)✅ (view plot)75,390.00 (-0.22%)76,438.10 (98.63%)
distance_matrix_euclidean/window: Some(10), parallelize: false✅ (view plot)2,995,300,000.00 (+11.38%)6,292,236,741.20 (47.60%)
distance_matrix_euclidean/window: Some(10), parallelize: true✅ (view plot)2,994,100,000.00 (+11.36%)6,289,797,547.17 (47.60%)
distance_matrix_euclidean/window: Some(2), parallelize: false✅ (view plot)537,100,000.00 (-32.53%)1,963,184,378.88 (27.36%)
distance_matrix_euclidean/window: Some(2), parallelize: true✅ (view plot)536,620,000.00 (-32.59%)1,963,206,933.86 (27.33%)
fit/air_passengers✅ (view plot)424,130.00 (-2.23%)441,831.04 (95.99%)
forecast/air_passengers✅ (view plot)1,413.00 (+0.41%)1,510.23 (93.56%)
season_eight✅ (view plot)22,250.00 (+3.54%)22,340.45 (99.60%)
vic_elec✅ (view plot)39,353,000.00 (+2.96%)39,617,079.02 (99.33%)

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

@sd2k sd2k changed the title dtw Add augurs-dtw crate with dynamic time warping implementation Jul 16, 2024
@sd2k sd2k marked this pull request as ready for review July 19, 2024 09:19
@sd2k sd2k requested a review from gitdoluquita July 19, 2024 09:20
@sd2k sd2k changed the title Add augurs-dtw crate with dynamic time warping implementation feat: add augurs-dtw crate with dynamic time warping implementation Jul 19, 2024
Copy link
Contributor

coderabbitai bot commented Jul 19, 2024

Walkthrough

The recent updates significantly enhance the augurs library by integrating the augurs-dtw crate, which implements the Dynamic Time Warping (DTW) algorithm for time series analysis. Key improvements include new modules for DTW functionality, a robust distance matrix implementation, and extensive documentation. This integration streamlines customizable distance computations, enabling improved performance and usability across diverse projects.

Changes

Files Change Summary
Cargo.toml, README.md, crates/.../Cargo.toml Added dependency on augurs-dtw, updated profiles, and included DTW package documentation.
crates/augurs-core/src/distance.rs, crates/augurs-dtw/src/lib.rs Introduced DistanceMatrix struct with error handling and iterator functionality for distance computations.
crates/augurs-dtw/... Added files for DTW algorithm implementation, including benchmarks and licensing information, enhancing usability and compliance.
crates/pyaugurs/..., crates/augurs-js/... Included DTW functionality in Python bindings, enhancing interoperability and usability for users.
crates/pyaugurs/src/distance.rs, crates/pyaugurs/src/dtw.rs Implemented wrappers for distance calculations and conversions, facilitating seamless integration with Python.
crates/pyaugurs/src/seasons.rs Improved documentation for seasonalities function, clarifying parameter usage and constraints.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant PyAugurs
    participant AugursDtw
    participant DistanceMatrix

    User->>PyAugurs: Call DTW Function
    PyAugurs->>AugursDtw: Initialize Dtw instance
    AugursDtw->>DistanceMatrix: Compute distance matrix
    DistanceMatrix-->>AugursDtw: Return distances
    AugursDtw-->>PyAugurs: Return results
    PyAugurs-->>User: Display DTW results
Loading

🐇 "In the garden where numbers twine,
A dance of time, both yours and mine.
With DTW's grace, we hop along,
Finding patterns in the song.
Let distances fade, as we calculate,
For every sequence, there’s hope—don’t wait!" 🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

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>.
    • Generate unit testing code for this file.
    • 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 generate unit testing code for this file.
    • @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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @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.

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 as 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 resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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.

Copy link
Contributor

@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: 4

Outside diff range, codebase verification and nitpick comments (6)
crates/augurs-dtw/CHANGELOG.md (1)

7-10: Consider using "Others" instead of "Other".

The plural noun “others” fits better in this context.

- ### Other
+ ### Others
Tools
LanguageTool

[misspelling] ~9-~9: It seems that the plural noun “others” fits better in this context.
Context: ...pec/v2.0.0.html). ## [Unreleased] ### Other - Add augurs-dtw crate

(OTHER_OTHERS)

crates/pyaugurs/README.md (1)

64-74: DTW section looks good.

The example is clear and demonstrates the usage of the DTW functionality effectively.

Specify the language for the code block.

To improve readability and syntax highlighting, specify the language for the code block.

- ```python

</blockquote></details>
<details>
<summary>crates/augurs-dtw/README.md (2)</summary><blockquote>

`24-33`: **Usage section looks good.**

The example is clear and demonstrates the usage of the `augurs_dtw` crate effectively.


**Specify the language for the code block.**

To improve readability and syntax highlighting, specify the language for the code block.

```diff
- ```
+ ```rust

<details>
<summary>Tools</summary>

<details>
<summary>Markdownlint</summary><blockquote>

26-26: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</blockquote></details>

</details>

---

`35-39`: **Credits section looks good.**

The content is clear and acknowledges the UCR Suite appropriately.


**Fix the hyphenation issue.**

The term "full featured" should be hyphenated.

```diff
- full featured
+ full-featured
Tools
LanguageTool

[misspelling] ~38-38: This word is normally spelled with a hyphen.
Context: ...t note that dtaidistance is much more full featured! [ucr-suite]: https://www.cs.ucr.edu/
...

(EN_COMPOUNDS_FULL_FEATURED)

crates/augurs-dtw/src/lib.rs (2)

93-362: LGTM! Consider adding comments to the distance method for clarity.

The Dtw struct and its methods are well-defined. However, the distance method contains a complex loop that might benefit from additional comments for clarity.


454-513: LGTM! Consider adding comments to the parallel computation for clarity.

The methods for computing the distance matrix are well-defined. However, the parallel computation might benefit from additional comments for clarity.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b47e283 and 6e4bd7f.

Files ignored due to path filters (1)
  • crates/augurs-dtw/data/series.csv is excluded by !**/*.csv
Files selected for processing (22)
  • Cargo.toml (2 hunks)
  • README.md (2 hunks)
  • crates/augurs-core/src/distance.rs (1 hunks)
  • crates/augurs-core/src/lib.rs (1 hunks)
  • crates/augurs-dtw/CHANGELOG.md (1 hunks)
  • crates/augurs-dtw/Cargo.toml (1 hunks)
  • crates/augurs-dtw/LICENSE-APACHE (1 hunks)
  • crates/augurs-dtw/LICENSE-MIT (1 hunks)
  • crates/augurs-dtw/README.md (1 hunks)
  • crates/augurs-dtw/benches/dtw.rs (1 hunks)
  • crates/augurs-dtw/src/lib.rs (1 hunks)
  • crates/augurs-js/Cargo.toml (1 hunks)
  • crates/augurs-js/src/dtw.rs (1 hunks)
  • crates/augurs-js/src/lib.rs (1 hunks)
  • crates/augurs-mstl/src/lib.rs (1 hunks)
  • crates/pyaugurs/Cargo.toml (1 hunks)
  • crates/pyaugurs/README.md (1 hunks)
  • crates/pyaugurs/augurs.pyi (4 hunks)
  • crates/pyaugurs/src/distance.rs (1 hunks)
  • crates/pyaugurs/src/dtw.rs (1 hunks)
  • crates/pyaugurs/src/lib.rs (2 hunks)
  • crates/pyaugurs/src/seasons.rs (1 hunks)
Files skipped from review due to trivial changes (2)
  • crates/augurs-dtw/LICENSE-APACHE
  • crates/augurs-dtw/LICENSE-MIT
Additional context used
LanguageTool
crates/augurs-dtw/CHANGELOG.md

[misspelling] ~9-~9: It seems that the plural noun “others” fits better in this context.
Context: ...pec/v2.0.0.html). ## [Unreleased] ### Other - Add augurs-dtw crate

(OTHER_OTHERS)

crates/augurs-dtw/README.md

[misspelling] ~38-38: This word is normally spelled with a hyphen.
Context: ...t note that dtaidistance is much more full featured! [ucr-suite]: https://www.cs.ucr.edu/
...

(EN_COMPOUNDS_FULL_FEATURED)

Markdownlint
crates/augurs-dtw/README.md

26-26: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

README.md

25-25: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style

(MD055, table-pipe-style)


25-25: Expected: 3; Actual: 2; Too few cells, row will be missing data
Table column count

(MD056, table-column-count)

Additional comments not posted (51)
crates/augurs-dtw/CHANGELOG.md (2)

1-2: Changelog format looks good.

The changelog format is clear and follows the "Keep a Changelog" convention.


4-5: Adherence to Semantic Versioning is noted.

The project adheres to Semantic Versioning, which is a good practice for version management.

crates/augurs-dtw/Cargo.toml (5)

1-10: Package metadata looks good.

The package metadata is well-structured and follows Rust conventions.


12-15: Dependencies are well-defined.

The dependencies section is clear and includes necessary dependencies like augurs-core and rayon.


16-18: Feature flag for parallel processing.

The feature flag for parallel processing using rayon is a good addition.


19-22: Development dependencies are well-defined.

The development dependencies section is clear and includes necessary dependencies like criterion and itertools.


23-28: Library and benchmark settings look good.

The library and benchmark settings are well-defined and follow Rust conventions.

crates/pyaugurs/Cargo.toml (1)

20-20: New dependency augurs-dtw added.

The addition of augurs-dtw as a dependency is noted and looks correct.

crates/augurs-core/src/lib.rs (2)

15-15: Module addition approved.

The distance module is correctly added to the list of modules.


22-22: Public export approved.

The DistanceMatrix type is correctly made publicly accessible.

crates/augurs-js/Cargo.toml (1)

23-23: Dependency addition approved.

The augurs-dtw dependency is correctly added to the Cargo.toml file.

crates/pyaugurs/src/seasons.rs (1)

9-16: Documentation and parameter descriptions approved.

The documentation and parameter descriptions provide clear and accurate information about the function's parameters and expected outputs.

Cargo.toml (3)

27-27: Dependency addition looks good.

The new dependency augurs-dtw is correctly specified.


50-52: Profile settings for release look good.

The settings codegen-units = 1 and lto = false are appropriate for optimizing the build process.


54-56: Profile settings for profiling look good.

The settings inherits = "release" and debug = true are appropriate for performance analysis.

crates/augurs-dtw/README.md (2)

1-21: Introduction and features section looks good.

The overview and features list are clear and informative.


44-47: License section looks good.

The dual-licensing information is clear and accurate.

crates/pyaugurs/src/distance.rs (4)

1-9: Documentation comments and imports look good.

The documentation comments provide clear context for the module and struct. The imports are necessary and correctly specified.


10-30: Struct definition and trait implementations look good.

The DistanceMatrix struct wraps around augurs_core::DistanceMatrix. The From trait is implemented correctly for conversions between DistanceMatrix and augurs_core::DistanceMatrix.


32-54: pymethods implementation looks good.

The __repr__ method provides a useful string representation. The to_numpy method correctly converts the nested Vec<Vec<f64>> to a 2D numpy array, with considerations for potential optimizations.


57-61: Additional methods implementation looks good.

The inner method is straightforward and correctly implemented to access the inner augurs_core::DistanceMatrix.

crates/augurs-js/src/lib.rs (1)

17-17: Module addition looks good.

The addition of the dtw module is straightforward and aligns with the PR objectives.

crates/augurs-dtw/benches/dtw.rs (4)

1-20: Imports and examples function look good.

The imports bring in necessary dependencies for benchmarking and data manipulation. The examples function correctly reads and parses the CSV file, handling potential NaN values.


22-42: distance_euclidean benchmarking function looks good.

The function is well-structured and correctly benchmarks the Euclidean distance calculation for different window sizes.


44-68: distance_matrix_euclidean benchmarking function looks good.

The function is well-structured and correctly benchmarks the distance matrix calculation for different window sizes and parallelization options.


70-71: Criterion group and main function look good.

The criterion group and main function are correctly defined to include the benchmarking functions.

crates/augurs-core/src/distance.rs (4)

3-8: LGTM!

The DistanceMatrixError enum is well-defined and correctly implements fmt::Display and std::error::Error.


62-74: LGTM!

The Index implementations for DistanceMatrix are correct and follow Rust conventions.


76-82: LGTM!

The IntoIterator implementation for DistanceMatrix is correct and follows Rust conventions.


84-95: LGTM!

The DistanceMatrixIter struct and its Iterator implementation are correct and follow Rust conventions.

crates/pyaugurs/augurs.pyi (2)

56-60: LGTM!

The updates to use double quotes for type hints in the MSTL class align with Python's type hinting conventions.


78-78: LGTM!

The DistanceFn type alias is well-defined and improves type safety.

crates/pyaugurs/src/lib.rs (2)

17-17: LGTM!

The distance module is correctly introduced and registered in the augurs function.


18-18: LGTM!

The dtw module is correctly introduced and registered in the augurs function.

README.md (1)

66-66: LGTM!

The new link for augurs-dtw is correctly added.

crates/augurs-js/src/dtw.rs (6)

3-10: LGTM!

The imports are necessary and correctly included.


12-31: LGTM!

The InnerDtw enum and its methods are well-structured and provide necessary functionality for DTW calculations.


33-40: LGTM!

The fmt::Debug implementation for InnerDtw is correct and provides useful debug information.


42-74: LGTM!

The DtwOpts struct is well-defined and the use of serde and tsify attributes is appropriate.


76-98: LGTM!

The DistanceMatrix struct and its conversion implementations are correct and provide necessary functionality.


100-169: LGTM!

The Dtw struct and its methods are well-structured and provide necessary functionality for DTW calculations.

crates/pyaugurs/src/dtw.rs (5)

3-10: LGTM!

The imports are necessary and correctly included.


12-66: LGTM!

The InnerDtw enum and its methods are well-structured and provide necessary functionality for DTW calculations.


68-75: LGTM!

The fmt::Debug implementation for InnerDtw is correct and provides useful debug information.


77-96: LGTM!

The DistanceFn enum and its implementation are correct and provide necessary functionality for selecting distance functions.


98-201: LGTM!

The Dtw struct and its methods are well-structured and provide necessary functionality for DTW calculations.

crates/augurs-mstl/src/lib.rs (1)

95-97: Verify the correctness of the parameter type change.

The parameter type has been updated to stlrs::MstlParams. Ensure that this change is consistent with the rest of the codebase and that all references to MstlParams are updated accordingly.

Verification successful

Verification successful: Parameter type change is consistent.

The parameter type MstlParams has been updated to stlrs::MstlParams across the relevant files, ensuring consistency in the codebase.

  • crates/augurs-mstl/src/lib.rs
  • crates/augurs-mstl/benches/vic_elec.rs
  • crates/augurs-mstl/benches/vic_elec_iai.rs
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `MstlParams` are updated to `stlrs::MstlParams`.

# Test: Search for the usage of `MstlParams`. Expect: Only occurances of `stlrs::MstlParams`.
rg --type rust -A 5 $'MstlParams'

Length of output: 3421

crates/augurs-dtw/src/lib.rs (4)

1-10: LGTM! Documentation and imports are clear.

The documentation is well-written and the necessary imports are included.


14-91: LGTM! Distance trait and its implementations are well-defined.

The Distance trait and its implementations for Euclidean and Manhattan distances are well-defined and include necessary optimizations.


364-452: LGTM! Early stopping methods are well-defined.

The methods for early stopping in the DTW computation are well-defined and include necessary optimizations.


516-652: LGTM! Tests are comprehensive.

The tests for the DTW implementation are comprehensive and cover various scenarios, including edge cases.

Comment on lines +50 to +59
/// Returns the shape of the matrix.
///
/// The first element is the number of rows and the second element
/// is the number of columns.
///
/// The matrix is square, so the number of rows is equal to the number of columns
/// and the number of input series.
pub fn shape(&self) -> (usize, usize) {
(self.matrix.len(), self.matrix.len())
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Simplify the shape method.

The shape method can be simplified by returning the length of the matrix directly.

-  (self.matrix.len(), self.matrix.len())
+  let size = self.matrix.len();
+  (size, size)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// Returns the shape of the matrix.
///
/// The first element is the number of rows and the second element
/// is the number of columns.
///
/// The matrix is square, so the number of rows is equal to the number of columns
/// and the number of input series.
pub fn shape(&self) -> (usize, usize) {
(self.matrix.len(), self.matrix.len())
}
/// Returns the shape of the matrix.
///
/// The first element is the number of rows and the second element
/// is the number of columns.
///
/// The matrix is square, so the number of rows is equal to the number of columns
/// and the number of input series.
pub fn shape(&self) -> (usize, usize) {
let size = self.matrix.len();
(size, size)
}

Comment on lines +24 to +36
impl DistanceMatrix {
/// Create a new `DistanceMatrix` from a square matrix.
///
/// # Errors
///
/// Returns an error if the input matrix is not square.
pub fn try_from_square(matrix: Vec<Vec<f64>>) -> Result<Self, DistanceMatrixError> {
if matrix.iter().all(|x| x.len() == matrix.len()) {
Ok(Self { matrix })
} else {
Err(DistanceMatrixError::InvalidDistanceMatrix)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Optimize the try_from_square method.

The try_from_square method can be optimized by checking the length of the matrix once and comparing it with each row's length.

-  if matrix.iter().all(|x| x.len() == matrix.len()) {
+  let size = matrix.len();
+  if matrix.iter().all(|x| x.len() == size) {
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
impl DistanceMatrix {
/// Create a new `DistanceMatrix` from a square matrix.
///
/// # Errors
///
/// Returns an error if the input matrix is not square.
pub fn try_from_square(matrix: Vec<Vec<f64>>) -> Result<Self, DistanceMatrixError> {
if matrix.iter().all(|x| x.len() == matrix.len()) {
Ok(Self { matrix })
} else {
Err(DistanceMatrixError::InvalidDistanceMatrix)
}
}
impl DistanceMatrix {
/// Create a new `DistanceMatrix` from a square matrix.
///
/// # Errors
///
/// Returns an error if the input matrix is not square.
pub fn try_from_square(matrix: Vec<Vec<f64>>) -> Result<Self, DistanceMatrixError> {
let size = matrix.len();
if matrix.iter().all(|x| x.len() == size) {
Ok(Self { matrix })
} else {
Err(DistanceMatrixError::InvalidDistanceMatrix)
}
}

Copy link
Contributor

@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: 7

Outside diff range, codebase verification and nitpick comments (3)
crates/augurs-dtw/CHANGELOG.md (1)

9-9: Consider using "Others" instead of "Other".

The plural noun “Others” fits better in this context.

- ### Other
+ ### Others
Tools
LanguageTool

[misspelling] ~9-~9: It seems that the plural noun “others” fits better in this context.
Context: ...pec/v2.0.0.html). ## [Unreleased] ### Other - Add augurs-dtw crate

(OTHER_OTHERS)

crates/augurs-dtw/README.md (1)

26-33: Specify the language for the code block.

Add a language identifier to the fenced code block to improve syntax highlighting.

-```
+```rust
Tools
Markdownlint

26-26: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

crates/pyaugurs/src/distance.rs (1)

1-61: Add more comments for better clarity.

Consider adding more comments to the code to explain the logic and purpose of each method.

+ // Wrapper around the `augurs_core` distance matrix type.
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6e4bd7f and 39509cf.

Files ignored due to path filters (1)
  • crates/augurs-dtw/data/series.csv is excluded by !**/*.csv
Files selected for processing (22)
  • Cargo.toml (2 hunks)
  • README.md (2 hunks)
  • crates/augurs-core/src/distance.rs (1 hunks)
  • crates/augurs-core/src/lib.rs (1 hunks)
  • crates/augurs-dtw/CHANGELOG.md (1 hunks)
  • crates/augurs-dtw/Cargo.toml (1 hunks)
  • crates/augurs-dtw/LICENSE-APACHE (1 hunks)
  • crates/augurs-dtw/LICENSE-MIT (1 hunks)
  • crates/augurs-dtw/README.md (1 hunks)
  • crates/augurs-dtw/benches/dtw.rs (1 hunks)
  • crates/augurs-dtw/src/lib.rs (1 hunks)
  • crates/augurs-js/Cargo.toml (1 hunks)
  • crates/augurs-js/src/dtw.rs (1 hunks)
  • crates/augurs-js/src/lib.rs (1 hunks)
  • crates/augurs-mstl/src/lib.rs (1 hunks)
  • crates/pyaugurs/Cargo.toml (1 hunks)
  • crates/pyaugurs/README.md (1 hunks)
  • crates/pyaugurs/augurs.pyi (4 hunks)
  • crates/pyaugurs/src/distance.rs (1 hunks)
  • crates/pyaugurs/src/dtw.rs (1 hunks)
  • crates/pyaugurs/src/lib.rs (2 hunks)
  • crates/pyaugurs/src/seasons.rs (1 hunks)
Files skipped from review due to trivial changes (4)
  • crates/augurs-dtw/Cargo.toml
  • crates/augurs-dtw/LICENSE-APACHE
  • crates/augurs-dtw/LICENSE-MIT
  • crates/augurs-mstl/src/lib.rs
Additional context used
LanguageTool
crates/augurs-dtw/CHANGELOG.md

[misspelling] ~9-~9: It seems that the plural noun “others” fits better in this context.
Context: ...pec/v2.0.0.html). ## [Unreleased] ### Other - Add augurs-dtw crate

(OTHER_OTHERS)

crates/augurs-dtw/README.md

[misspelling] ~38-38: This word is normally spelled with a hyphen.
Context: ...t note that dtaidistance is much more full featured! [ucr-suite]: https://www.cs.ucr.edu/
...

(EN_COMPOUNDS_FULL_FEATURED)

Markdownlint
crates/augurs-dtw/README.md

26-26: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

README.md

25-25: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style

(MD055, table-pipe-style)


25-25: Expected: 3; Actual: 2; Too few cells, row will be missing data
Table column count

(MD056, table-column-count)

Additional comments not posted (39)
crates/pyaugurs/Cargo.toml (1)

20-20: Dependency addition approved.

The addition of the augurs-dtw crate as a workspace dependency aligns with the PR objectives.

crates/augurs-core/src/lib.rs (2)

15-15: New module addition approved.

The addition of the distance module is consistent with the PR objectives and expands the library's capabilities.


22-22: Export addition approved.

The export of DistanceMatrix from the distance module aligns with the PR objectives and enhances the library's public API.

crates/augurs-js/Cargo.toml (1)

23-23: LGTM! Verify the usage of the new dependency.

The addition of the augurs-dtw dependency looks good.

Ensure that the new dependency is correctly utilized in the codebase.

Verification successful

Let's expand our search to include all file types to ensure we capture any indirect usage or configuration references of augurs-dtw.


To ensure the new dependency augurs-dtw is correctly utilized, let's search for function calls or module imports related to augurs-dtw in the Rust codebase.


Verified: The augurs-dtw dependency is correctly utilized in the codebase.

The dependency is referenced and used in multiple Rust files, confirming its integration and functionality.

  • crates/augurs-dtw/src/lib.rs
  • crates/augurs-dtw/benches/dtw.rs
  • crates/pyaugurs/src/dtw.rs
  • crates/augurs-js/src/dtw.rs
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the new dependency `augurs-dtw`.

# Test: Search for the usage of `augurs-dtw` in the codebase.
rg --type rust -A 5 $'augurs-dtw'

Length of output: 32


Script:

#!/bin/bash
# Description: Verify the usage of the new dependency `augurs-dtw` in the entire codebase.

# Test: Search for the usage of `augurs-dtw` in all file types.
rg -A 5 'augurs-dtw'

Length of output: 2999


Script:

#!/bin/bash
# Description: Verify the usage of the new dependency `augurs-dtw` in the Rust codebase.

# Test: Search for function calls or module imports related to `augurs-dtw` in the Rust codebase.
rg 'augurs_dtw' --type rust -A 5

Length of output: 4472

crates/pyaugurs/src/seasons.rs (1)

9-17: LGTM! Documentation improvements are clear and accurate.

The detailed parameter descriptions and return value clarifications enhance the usability and understanding of the seasonalities function.

Cargo.toml (3)

50-53: LGTM! Build optimization settings look appropriate.

The addition of the [profile.release] section with codegen-units = 1 and lto = false is intended to optimize the build process.


54-56: LGTM! Profiling settings look appropriate.

The addition of the [profile.profiling] section with inherits = "release" and debug = true is intended to enable profiling with debugging information.


27-27: LGTM! Verify the usage of the new dependency.

The addition of the augurs-dtw dependency looks good.

Ensure that the new dependency is correctly utilized in the codebase.

Verification successful

The new dependency augurs-dtw is correctly utilized in the codebase.

The dependency is imported and used in several files, indicating proper integration.

  • crates/augurs-dtw/src/lib.rs
  • crates/augurs-dtw/benches/dtw.rs
  • crates/pyaugurs/src/dtw.rs
  • crates/augurs-js/src/dtw.rs
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the new dependency `augurs-dtw`.

# Test: Search for the usage of `augurs-dtw` in the codebase.
rg --type rust -A 5 $'augurs-dtw'

Length of output: 32


Script:

#!/bin/bash
# Description: Verify the usage of the new dependency `augurs-dtw`.

# Search for the import of `augurs-dtw` in the Rust codebase.
rg --type rust 'use augurs_dtw'

# Search for function calls that might relate to `augurs-dtw` in the Rust codebase.
rg --type rust 'augurs_dtw::'

Length of output: 1569

crates/pyaugurs/README.md (1)

64-74: Verify the import statement and example code.

Ensure that the augurs module and Dtw class are correctly imported and used.

Additionally, consider adding comments to the code snippets for better clarity.

Verification successful

The import statement and example code for DTW are correct.

The Dtw class and its methods distance and distance_matrix are correctly defined in the crates/pyaugurs/augurs.pyi file.

  • crates/pyaugurs/augurs.pyi: class Dtw, def distance, def distance_matrix

Consider adding comments to the code snippets for better clarity.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the import statement and example code for DTW.

# Test: Search for the `Dtw` class in the `augurs` module. Expect: Class definition.
rg --type python -A 5 $'class Dtw'

Length of output: 66


Script:

#!/bin/bash
# Description: Verify the import statement and example code for DTW.

# Test: Search for the `Dtw` class in Python files. Expect: Class definition.
rg 'class Dtw' --glob '*.py' -A 5

Length of output: 184


Script:

#!/bin/bash
# Description: Verify the import statement and example code for DTW.

# Test: Search for the `Dtw` class in all files. Expect: Class definition.
rg 'class Dtw' -A 5

Length of output: 283


Script:

#!/bin/bash
# Description: Verify the methods in the `Dtw` class.

# Test: Search for the `distance` and `distance_matrix` methods in the `Dtw` class.
rg 'def distance' -A 5 crates/pyaugurs/augurs.pyi
rg 'def distance_matrix' -A 5 crates/pyaugurs/augurs.pyi

Length of output: 671

crates/augurs-dtw/README.md (1)

26-33: Verify the example code.

Ensure that the example code for DTW is correct and functional.

Verification successful

The example code for DTW is correct and functional.

The Dtw struct and its methods euclidean and distance are correctly implemented in the augurs-dtw crate.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the example code for DTW.

# Test: Search for the `Dtw` struct in the `augurs_dtw` crate. Expect: Struct definition.
rg --type rust -A 5 $'struct Dtw'

Length of output: 1255


Script:

#!/bin/bash
# Description: Verify the example code for DTW.

# Test: Search for the `impl` block of `Dtw` in the `augurs-dtw` crate to check for `euclidean` and `distance` methods.
rg --type rust -A 10 'impl Dtw' crates/augurs-dtw/src/lib.rs

Length of output: 614


Script:

#!/bin/bash
# Description: Verify the example code for DTW.

# Test: Search for the `distance` method in the `Dtw` implementation in the `augurs-dtw` crate.
rg --type rust -A 10 'fn distance' crates/augurs-dtw/src/lib.rs

Length of output: 4345

Tools
Markdownlint

26-26: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

crates/pyaugurs/src/distance.rs (1)

46-54: Verify the conversion methods.

Ensure that the methods for converting the distance matrix to a numpy array are correct and efficient.

crates/augurs-js/src/lib.rs (1)

17-17: New module dtw added.

The new module dtw has been added. Ensure that it is correctly implemented and integrated with the rest of the library.

crates/augurs-core/src/distance.rs (5)

3-8: LGTM!

The DistanceMatrixError enum is correctly implemented.


10-14: LGTM!

The fmt::Display implementation for DistanceMatrixError is correctly implemented.


16-16: LGTM!

The std::error::Error implementation for DistanceMatrixError is correctly implemented.


18-22: LGTM!

The DistanceMatrix struct is correctly implemented.


84-95: LGTM!

The DistanceMatrixIter struct and implementation are correctly implemented.

crates/pyaugurs/augurs.pyi (1)

56-60: LGTM! The changes improve readability and consistency.

The modifications to the method signatures in the MSTL class enhance readability and maintain consistency in string literals.

crates/pyaugurs/src/lib.rs (4)

17-17: LGTM! The addition of the distance module enhances functionality.

The new distance module adds valuable features to the library.


18-18: LGTM! The addition of the dtw module enhances functionality.

The new dtw module adds valuable features to the library.


116-116: LGTM! The registration of DistanceMatrix ensures availability in Python.

The DistanceMatrix class is now accessible in the Python environment.


117-117: LGTM! The registration of Dtw ensures availability in Python.

The Dtw class is now accessible in the Python environment.

crates/augurs-js/src/dtw.rs (6)

1-10: Good use of imports and module documentation.

The imports are well-organized and the module documentation is clear. This helps in understanding the purpose of the file.


12-15: Enum InnerDtw is well-defined.

The InnerDtw enum encapsulates different DTW implementations, making it extensible for future distance functions.


17-31: InnerDtw methods are correctly implemented.

The methods for calculating distance and distance matrix are straightforward and correctly implemented.


42-74: DtwOpts struct is well-defined.

The DtwOpts struct provides options for DTW calculation with optional fields, and uses appropriate attributes for serialization and deserialization.


76-98: DistanceMatrix struct and its implementations are correctly defined.

The struct is opaque and intended for further processing within augurs. The conversion implementations are correctly defined.


100-169: Dtw struct and its methods are well-defined.

The methods cover the necessary functionality for DTW calculations, including creating instances using different distance functions and calculating distances.

crates/pyaugurs/src/dtw.rs (5)

1-10: Good use of imports and module documentation.

The imports are well-organized and the module documentation is clear. This helps in understanding the purpose of the file.


12-15: Enum InnerDtw is well-defined.

The InnerDtw enum encapsulates different DTW implementations, making it extensible for future distance functions.


17-66: InnerDtw methods are correctly implemented.

The methods for setting options and calculating distance and distance matrix are straightforward and correctly implemented.


77-96: DistanceFn enum and its implementation are correctly defined.

The enum provides different distance functions and the FromStr implementation allows for easy parsing from strings.


98-202: Dtw struct and its methods are well-defined.

The methods cover the necessary functionality for DTW calculations, including creating instances using different distance functions and calculating distances.

crates/augurs-dtw/src/lib.rs (6)

1-12: Crate-level documentation and imports are well-defined.

The documentation provides a good overview of the crate and the imports are correctly defined.


14-91: Distance trait and its implementations are well-defined and optimized.

The trait defines the interface for distance functions and the implementations for Euclidean and Manhattan distances are optimized for performance.


93-362: Dtw struct and its methods are well-defined.

The methods cover the necessary functionality for DTW calculations, including creating instances using different distance functions and calculating distances.


364-452: distance_with_early_stopping method and related helper functions are correctly implemented.

The method and helper functions provide useful optimizations for DTW calculations.


454-513: distance_matrix method is well-defined and provides useful functionality.

The method calculates the distance matrix for a set of series and supports parallel computation.


516-653: Tests are comprehensive and cover necessary scenarios.

The tests cover various scenarios for DTW calculations, including different distance functions, edge cases, and distance matrix calculations.

Comment on lines +80 to +101
class Dtw:
"""
Dynamic Time Warping (DTW) distance calculations.
"""

def __init__(
self, window: int = 3, distance_fn: DistanceFn = "euclidean"
) -> None: ...
def distance(
self,
a: npt.NDArray[np.float64],
b: npt.NDArray[np.float64],
) -> float: ...
"""
Compute the distance between two time series using DTW.
"""
def distance_matrix(
self, series: list[npt.NDArray[np.float64]]
) -> npt.NDArray[np.float64]: ...
"""
Compute the pairwise distance matrix between a list of time series using DTW.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Move docstrings inside the method bodies.

The docstrings for the methods in the Dtw class should be moved inside the method bodies to follow Python conventions.

-  """
-  Dynamic Time Warping (DTW) distance calculations.
-  """
+  def __init__(
+      self, window: int = 3, distance_fn: DistanceFn = "euclidean"
+  ) -> None:
+      """
+      Initialize the DTW instance.
+      """
+  def distance(
+      self,
+      a: npt.NDArray[np.float64],
+      b: npt.NDArray[np.float64],
+  ) -> float:
+      """
+      Compute the distance between two time series using DTW.
+      """
+  def distance_matrix(
+      self, series: list[npt.NDArray[np.float64]]
+  ) -> npt.NDArray[np.float64]:
+      """
+      Compute the pairwise distance matrix between a list of time series using DTW.
+      """
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
class Dtw:
"""
Dynamic Time Warping (DTW) distance calculations.
"""
def __init__(
self, window: int = 3, distance_fn: DistanceFn = "euclidean"
) -> None: ...
def distance(
self,
a: npt.NDArray[np.float64],
b: npt.NDArray[np.float64],
) -> float: ...
"""
Compute the distance between two time series using DTW.
"""
def distance_matrix(
self, series: list[npt.NDArray[np.float64]]
) -> npt.NDArray[np.float64]: ...
"""
Compute the pairwise distance matrix between a list of time series using DTW.
"""
class Dtw:
def __init__(
self, window: int = 3, distance_fn: DistanceFn = "euclidean"
) -> None:
"""
Initialize the DTW instance.
"""
def distance(
self,
a: npt.NDArray[np.float64],
b: npt.NDArray[np.float64],
) -> float:
"""
Compute the distance between two time series using DTW.
"""
def distance_matrix(
self, series: list[npt.NDArray[np.float64]]
) -> npt.NDArray[np.float64]:
"""
Compute the pairwise distance matrix between a list of time series using DTW.
"""

## Credits

The algorithm is based on the code from the [UCR Suite][ucr-suite]. Benchmarks show similar
or faster timings compared to [`dtaidistance`]'s C implementation, but note that `dtaidistance` is much more
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct the spelling.

The word "full-featured" is normally spelled with a hyphen.

- full featured
+ full-featured
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
or faster timings compared to [`dtaidistance`]'s C implementation, but note that `dtaidistance` is much more
or faster timings compared to [`dtaidistance`]'s C implementation, but note that `dtaidistance` is much more full-featured!
Tools
LanguageTool

[misspelling] ~38-38: This word is normally spelled with a hyphen.
Context: ...t note that dtaidistance is much more full featured! [ucr-suite]: https://www.cs.ucr.edu/
...

(EN_COMPOUNDS_FULL_FEATURED)

Comment on lines +44 to +68
fn distance_matrix_euclidean(c: &mut Criterion) {
let mut group = c.benchmark_group("distance_matrix_euclidean");
let examples = examples();
let examples = examples.iter().map(|v| v.as_slice()).collect::<Vec<_>>();
let windows = [Some(2), Some(10)];
let parallelize = [false, true];
for (window, parallelize) in windows.into_iter().cartesian_product(parallelize) {
group.bench_with_input(
BenchmarkId::from_parameter(format!(
"window: {:?}, parallelize: {:?}",
window, parallelize
)),
&examples,
|b, examples| {
b.iter(|| {
let mut dtw = Dtw::euclidean().parallelize(parallelize);
if let Some(window) = window {
dtw = dtw.with_window(window).with_max_distance(window as f64);
}
dtw.distance_matrix(examples)
});
},
);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve readability in distance_matrix_euclidean function.

The function correctly benchmarks the DTW distance matrix calculation, but readability can be improved.

fn distance_matrix_euclidean(c: &mut Criterion) {
    let mut group = c.benchmark_group("distance_matrix_euclidean");
    let examples = examples();
    let examples = examples.iter().map(|v| v.as_slice()).collect::<Vec<_>>();
    let windows = [Some(2), Some(10)];
    let parallelize = [false, true];
    for (window, parallelize) in windows.into_iter().cartesian_product(parallelize) {
        group.bench_with_input(
            BenchmarkId::from_parameter(format!(
                "window: {:?}, parallelize: {:?}",
                window, parallelize
            )),
            &examples,
            |b, examples| {
                b.iter(|| {
                    let mut dtw = Dtw::euclidean().parallelize(parallelize);
                    if let Some(window) = window {
                        dtw = dtw.with_window(window).with_max_distance(window as f64);
                    }
                    dtw.distance_matrix(examples)
                });
            },
        );
    }
}

Committable suggestion was skipped due to low confidence.

Comment on lines +6 to +20
fn examples() -> Vec<Vec<f64>> {
let raw = include_str!("../data/series.csv");
let n_columns = raw.lines().next().unwrap().split(',').count();
let n_rows = raw.lines().count();
let mut examples = vec![Vec::with_capacity(n_rows); n_columns];
for line in raw.lines() {
for (i, value) in line.split(',').enumerate() {
let value: f64 = value.parse().unwrap();
if !value.is_nan() {
examples[i].push(value);
}
}
}
examples
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve readability and error handling in examples function.

The function correctly loads and parses the CSV data, but readability and error handling can be improved.

fn examples() -> Vec<Vec<f64>> {
    let raw = include_str!("../data/series.csv");
    let lines: Vec<&str> = raw.lines().collect();
    let n_columns = lines.first().unwrap().split(',').count();
    let n_rows = lines.len();
    let mut examples = vec![Vec::with_capacity(n_rows); n_columns];
    for line in lines {
        for (i, value) in line.split(',').enumerate() {
            if let Ok(value) = value.parse::<f64>() {
                if !value.is_nan() {
                    examples[i].push(value);
                }
            }
        }
    }
    examples
}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn examples() -> Vec<Vec<f64>> {
let raw = include_str!("../data/series.csv");
let n_columns = raw.lines().next().unwrap().split(',').count();
let n_rows = raw.lines().count();
let mut examples = vec![Vec::with_capacity(n_rows); n_columns];
for line in raw.lines() {
for (i, value) in line.split(',').enumerate() {
let value: f64 = value.parse().unwrap();
if !value.is_nan() {
examples[i].push(value);
}
}
}
examples
}
fn examples() -> Vec<Vec<f64>> {
let raw = include_str!("../data/series.csv");
let lines: Vec<&str> = raw.lines().collect();
let n_columns = lines.first().unwrap().split(',').count();
let n_rows = lines.len();
let mut examples = vec![Vec::with_capacity(n_rows); n_columns];
for line in lines {
for (i, value) in line.split(',').enumerate() {
if let Ok(value) = value.parse::<f64>() {
if !value.is_nan() {
examples[i].push(value);
}
}
}
}
examples
}

Comment on lines +22 to +42
fn distance_euclidean(c: &mut Criterion) {
let mut group = c.benchmark_group("distance_euclidean");
let examples = examples();
let (s, t) = (&examples[0], &examples[1]);
let windows = [None, Some(2), Some(5), Some(10), Some(20), Some(50)];
for window in windows {
group.bench_with_input(
BenchmarkId::from_parameter(format!("{:?}", window)),
&(s, t),
|b, (s, t)| {
b.iter(|| {
let mut dtw = Dtw::euclidean();
if let Some(window) = window {
dtw = dtw.with_window(window);
}
dtw.distance(s, t)
});
},
);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve readability in distance_euclidean function.

The function correctly benchmarks the DTW distance calculation, but readability can be improved.

fn distance_euclidean(c: &mut Criterion) {
    let mut group = c.benchmark_group("distance_euclidean");
    let examples = examples();
    let (s, t) = (&examples[0], &examples[1]);
    let windows = [None, Some(2), Some(5), Some(10), Some(20), Some(50)];
    for window in windows {
        group.bench_with_input(
            BenchmarkId::from_parameter(format!("{:?}", window)),
            &(s, t),
            |b, (s, t)| {
                b.iter(|| {
                    let mut dtw = Dtw::euclidean();
                    if let Some(window) = window {
                        dtw = dtw.with_window(window);
                    }
                    dtw.distance(s, t)
                });
            },
        );
    }
}

Committable suggestion was skipped due to low confidence.

Comment on lines +24 to +60
impl DistanceMatrix {
/// Create a new `DistanceMatrix` from a square matrix.
///
/// # Errors
///
/// Returns an error if the input matrix is not square.
pub fn try_from_square(matrix: Vec<Vec<f64>>) -> Result<Self, DistanceMatrixError> {
if matrix.iter().all(|x| x.len() == matrix.len()) {
Ok(Self { matrix })
} else {
Err(DistanceMatrixError::InvalidDistanceMatrix)
}
}

/// Consumes the `DistanceMatrix` and returns the inner matrix.
pub fn into_inner(self) -> Vec<Vec<f64>> {
self.matrix
}

/// Returns an iterator over the rows of the matrix.
pub fn iter(&self) -> DistanceMatrixIter<'_> {
DistanceMatrixIter {
iter: self.matrix.iter(),
}
}

/// Returns the shape of the matrix.
///
/// The first element is the number of rows and the second element
/// is the number of columns.
///
/// The matrix is square, so the number of rows is equal to the number of columns
/// and the number of input series.
pub fn shape(&self) -> (usize, usize) {
(self.matrix.len(), self.matrix.len())
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Simplify the try_from_square and shape methods.

The try_from_square and shape methods can be simplified for readability and performance.

pub fn try_from_square(matrix: Vec<Vec<f64>>) -> Result<Self, DistanceMatrixError> {
    let size = matrix.len();
    if matrix.iter().all(|x| x.len() == size) {
        Ok(Self { matrix })
    } else {
        Err(DistanceMatrixError::InvalidDistanceMatrix)
    }
}

pub fn shape(&self) -> (usize, usize) {
    let size = self.matrix.len();
    (size, size)
}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
impl DistanceMatrix {
/// Create a new `DistanceMatrix` from a square matrix.
///
/// # Errors
///
/// Returns an error if the input matrix is not square.
pub fn try_from_square(matrix: Vec<Vec<f64>>) -> Result<Self, DistanceMatrixError> {
if matrix.iter().all(|x| x.len() == matrix.len()) {
Ok(Self { matrix })
} else {
Err(DistanceMatrixError::InvalidDistanceMatrix)
}
}
/// Consumes the `DistanceMatrix` and returns the inner matrix.
pub fn into_inner(self) -> Vec<Vec<f64>> {
self.matrix
}
/// Returns an iterator over the rows of the matrix.
pub fn iter(&self) -> DistanceMatrixIter<'_> {
DistanceMatrixIter {
iter: self.matrix.iter(),
}
}
/// Returns the shape of the matrix.
///
/// The first element is the number of rows and the second element
/// is the number of columns.
///
/// The matrix is square, so the number of rows is equal to the number of columns
/// and the number of input series.
pub fn shape(&self) -> (usize, usize) {
(self.matrix.len(), self.matrix.len())
}
}
impl DistanceMatrix {
/// Create a new `DistanceMatrix` from a square matrix.
///
/// # Errors
///
/// Returns an error if the input matrix is not square.
pub fn try_from_square(matrix: Vec<Vec<f64>>) -> Result<Self, DistanceMatrixError> {
let size = matrix.len();
if matrix.iter().all(|x| x.len() == size) {
Ok(Self { matrix })
} else {
Err(DistanceMatrixError::InvalidDistanceMatrix)
}
}
/// Consumes the `DistanceMatrix` and returns the inner matrix.
pub fn into_inner(self) -> Vec<Vec<f64>> {
self.matrix
}
/// Returns an iterator over the rows of the matrix.
pub fn iter(&self) -> DistanceMatrixIter<'_> {
DistanceMatrixIter {
iter: self.matrix.iter(),
}
}
/// Returns the shape of the matrix.
///
/// The first element is the number of rows and the second element
/// is the number of columns.
///
/// The matrix is square, so the number of rows is equal to the number of columns
/// and the number of input series.
pub fn shape(&self) -> (usize, usize) {
let size = self.matrix.len();
(size, size)
}
}

Copy link
Contributor

@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 39509cf and a2839ec.

Files selected for processing (2)
  • Cargo.toml (2 hunks)
  • README.md (2 hunks)
Files skipped from review as they are similar to previous changes (1)
  • Cargo.toml
Additional context used
Markdownlint
README.md

25-25: Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
Table pipe style

(MD055, table-pipe-style)


25-25: Expected: 3; Actual: 2; Too few cells, row will be missing data
Table column count

(MD056, table-column-count)

Additional comments not posted (1)
README.md (1)

66-66: LGTM!

The new reference link for augurs-dtw is correctly added.

README.md Outdated Show resolved Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@sd2k sd2k enabled auto-merge (squash) August 21, 2024 15:40
Copy link
Contributor

@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: CHILL

Commits

Files that changed from the base of the PR and between a2839ec and 694145e.

Files selected for processing (1)
  • README.md (2 hunks)
Additional comments not posted (1)
README.md (1)

25-25: Fix the table entry for augurs-dtw.

The new entry for augurs-dtw is missing the status column value and a trailing pipe.

The existing comment already addresses this issue.

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.

1 participant