[doc] Detailed docstring for functions in linalg module#32
[doc] Detailed docstring for functions in linalg module#32chaoming0625 merged 4 commits intomainfrom
Conversation
… reflect SaiUnit implementation
Reviewer's GuideThis pull request refactors and enhances the documentation for all linear algebra functions in the saiunit library to consistently support and demonstrate unit-aware computations using Quantity objects, updates function signatures and return types, and improves example code to reflect these changes. It also fixes the return value order for the eigh function and ensures all docstrings and examples are consistent with the new unit-handling paradigm. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @Routhleck - I've reviewed your changes - here's some feedback:
- The implementation of eig and eigh now returns (eigenvectors, eigenvalues) instead of the more common (eigenvalues, eigenvectors) order; consider clarifying this in the docstrings to avoid confusion for users.
- In the docstring for eigvalsh, the comment '# TODO: Seems wrong implementation' is present; please either address the concern or remove the comment if resolved.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| >>> import jax.numpy as jnp | ||
|
|
||
| >>> a = jnp.array([[1, -2j], | ||
| ... [2j, 1]]) |
There was a problem hiding this comment.
suggestion (performance): Avoid unnecessary eigenvector computation when only eigenvalues are needed
Re-add compute_left_eigenvectors=False and compute_right_eigenvectors=False to skip eigenvector computation and avoid unnecessary work.
| ... [2j, 1]]) | |
| return eig(a, compute_left_eigenvectors=False, | |
| compute_right_eigenvectors=False)[0] |
| if isinstance(x, Quantity): | ||
| w, v = lax.linalg.eigh(x.mantissa, lower=lower, symmetrize_input=symmetrize_input, | ||
| v, w = lax.linalg.eigh(x.mantissa, lower=lower, symmetrize_input=symmetrize_input, | ||
| sort_eigenvalues=sort_eigenvalues, subset_by_index=subset_by_index) | ||
| return maybe_decimal(Quantity(w, unit=x.unit)), v | ||
| return v, maybe_decimal(Quantity(w, unit=x.unit)) | ||
| else: | ||
| return lax.linalg.eigh(x, lower=lower, symmetrize_input=symmetrize_input, | ||
| sort_eigenvalues=sort_eigenvalues, subset_by_index=subset_by_index) |
There was a problem hiding this comment.
issue (code-quality): We've found these issues:
- Swap if/else branches (
swap-if-else-branches) - Remove unnecessary else after guard condition (
remove-unnecessary-else)
closes: #24
This pull request introduces updates to the
saiunitlibrary's linear algebra functions to enhance support for handling quantities with units. The changes include modifying function signatures, return types, and examples to reflect the use ofQuantityobjects, as well as improving consistency in documentation and examples.Updates to Linear Algebra Functions for Quantity Support
General Updates:
Quantityobjects instead of raw arrays. This change applies to the following functions:cholesky,solve,tensorsolve,lstsq,inv,pinv,tensorinv,norm,matrix_norm, andvector_norm. [1] [2] [3] [4] [5] [6] [7] [8] [9]Documentation and Examples:
saiunitimports and usage ofQuantityobjects, showcasing unit-aware computations. Examples now demonstrate the correct handling of units in operations like Cholesky decomposition, solving linear systems, and computing norms. [1] [2] [3] [4] [5]Function-Specific Changes:
eighFunction:(eigenvectors, eigenvalues)format.Norm Functions (
norm,matrix_norm,vector_norm):Quantityand adjusted examples to reflect unit-aware outputs. [1] [2] [3]These updates ensure that the library is consistent in its handling of quantities with units, improving usability and correctness for scientific and engineering applications.
Summary by Sourcery
Update linear algebra module documentation and examples to consistently use and demonstrate unit-aware Quantity objects, improving clarity and correctness for scientific applications.
Enhancements:
Documentation: