Skip to content

Document mdivide_left and mdivide_right - #985

Open
avehtari wants to merge 1 commit into
masterfrom
document-mdivide-left-right
Open

avehtari wants to merge 1 commit into
masterfrom
document-mdivide-left-right

Conversation

@avehtari

Copy link
Copy Markdown
Member

Submission Checklist

  • Builds locally (quarto render to HTML of matrix_operations.qmd, complex_matrix_operations.qmd and functions_index.qmd, Quarto 1.8.26)
  • New functions marked with <<{ since VERSION }>>
  • Declare copyright holder and open-source license: see below

Summary

Fixes #908.

Claude did this by getting function signatures from stanc3, and tested also that all signatures compile. Since versions Claude checked from git.

matrix_operations.qmd, section "Matrix division operators and functions":

  • New subsection "General matrix division functions" with mdivide_right(row_vector b, matrix A), mdivide_right(matrix B, matrix A), mdivide_left(matrix A, vector b) and mdivide_left(matrix A, matrix B). These are the function forms of / and \. All four signatures are in function_signatures.h of Stan 2.0.0, so they are marked since 2.0.
  • The two mdivide_left_spd entries had their return types swapped: (matrix A, vector b) showed matrix, and (matrix A, matrix B) showed vector. They now show vector and matrix, as in stanc.
  • The operator\ entries said "The left division of A by b". They now say "The left division of b by A", as the mdivide_left_tri_low and mdivide_left_spd entries do.

complex_matrix_operations.qmd, section "Complex matrix division operators and functions":

  • New subsection "Complex matrix division functions" with mdivide_right(complex_row_vector b, complex_matrix A) and mdivide_right(complex_matrix B, complex_matrix A). stanc 2.30.0 added these signatures together with the complex operator/, which is already documented, so they are marked since 2.30. There is no complex mdivide_left.

functions_index.qmd is regenerated with gen_index.py. The new and changed index entries link to the anchors that html-index.lua produces: I checked the SHA-1 of each new or changed signature comment against the regenerated index.

Check: each mdivide* signature in stanc --dump-stan-math-signatures (stanc3 v2.40.0) is now documented, and each documented mdivide* signature is in stanc.

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company):

Aalto University

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

@bob-carpenter

Copy link
Copy Markdown
Member

The PR itself looks fine, but I have a more general question about whether we want to expose this funciton.

Is there a reason to use mdivide_left and mdivide_right rather than \ and /? I think having two ways of writing the same thing is confusing, though we do it elsewhere with functions like square() and inverse().

Do we also document add(), multiply(), and so on for the other basic (elementwise) matrix operations?

@avehtari

Copy link
Copy Markdown
Member Author

I don't have opinion on this. I was just going through issues and making PRs for the easy ones (and adding to my TODO list some hard ones). I'm tagging @WardBrian who created the issue, and @spinkney who was mentioned as spotting this issue

@WardBrian

Copy link
Copy Markdown
Member

I believe these were spotted by @spinkney because some downstream tools build a list of functions in our docs and use it for syntax highlighting, and these functions did not get highlighted correctly since they were not documented.

That being said, @bob-carpenter makes two good points:

Is there a reason to use mdivide_left and mdivide_right rather than \ and /?

No, they generate exactly the same C++

Do we also document add(), multiply(), and so on for the other basic (elementwise) matrix operations?

Also no

@bob-carpenter

Copy link
Copy Markdown
Member

I think we should be consistent and either document all of these as named functions or none of them.

A middle ground might be to add a table in the operators section that lists the named equivalents. Most of these functions are hugely overloaded, so adding them all will be a lot of signatures to document.

This branch has not been deployed

No deployments
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.

Missing functions: mdivide_left, mdivide_right

3 participants