Skip to content

Conversation

@lilkeet
Copy link
Contributor

@lilkeet lilkeet commented Nov 10, 2025

Solves #683.
Not enough memory was being allocated for the iwork array which caused Lapack to sometimes write past the end of the sequence.
The natural log was used instead of binary log which was fine for certain tensor shapes since floor(ln(n)) == floor(log2(n)) for certain small n.
From Lapack's documentation:

!>          LWORK is INTEGER
!>          The dimension of the array WORK. LWORK must be at least 1.
!>          The exact minimum amount of workspace needed depends on M,
!>          N and NRHS. As long as LWORK is at least
!>              12*N + 2*N*SMLSIZ + 8*N*NLVL + N*NRHS + (SMLSIZ+1)**2,
!>          if M is greater than or equal to N or
!>              12*M + 2*M*SMLSIZ + 8*M*NLVL + M*NRHS + (SMLSIZ+1)**2,
!>          if M is less than N, the code will execute correctly.
!>          SMLSIZ is returned by ILAENV and is equal to the maximum
!>          size of the subproblems at the bottom of the computation
!>          tree (usually about 25), and
!>             NLVL = MAX( 0, INT( LOG_2( MIN( M,N )/(SMLSIZ+1) ) ) + 1 )
!>          For good performance, LWORK should generally be larger.
!>
!>          If LWORK = -1, then a workspace query is assumed; the routine
!>          only calculates the optimal size of the array WORK and the
!>          minimum size of the array IWORK, and returns these values as
!>          the first entries of the WORK and IWORK arrays, and no error
!>          message related to LWORK is issued by XERBLA.

@gemini-code-assist
Copy link

Summary of Changes

Hello @lilkeet, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical issue in the least squares solver's memory allocation for the iwork array by correcting the nlvl parameter calculation. By switching from natural logarithm to binary logarithm as specified by Lapack's documentation, the solver now accurately determines the required workspace, preventing potential memory overflows and ensuring robust operation.

Highlights

  • Corrected nlvl calculation: The calculation for nlvl (level of recursion) in the gelsd function was updated to use the binary logarithm (log2) instead of the natural logarithm (ln), aligning with Lapack's documentation for optimal workspace allocation.
  • Resolved iwork array memory allocation: The previous incorrect nlvl calculation could lead to insufficient memory allocation for the iwork array, potentially causing Lapack to write past allocated memory. This fix ensures correct memory provisioning.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses a critical bug in the gelsd wrapper for the least squares solver. The calculation for nlvl was incorrectly using the natural logarithm (ln) instead of the binary logarithm (log2), which is specified by the LAPACK documentation. This error could lead to insufficient memory allocation for the iwork array, risking memory corruption. The submitted change corrects the formula by using log2, which is the proper fix for this issue. The change is accurate and effectively resolves the bug.

Copy link
Owner

@mratsim mratsim left a comment

Choose a reason for hiding this comment

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

Excellent catch, thanks!

@mratsim mratsim merged commit 17ddeb4 into mratsim:master Nov 10, 2025
0 of 6 checks passed
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.

2 participants