-
Notifications
You must be signed in to change notification settings - Fork 7
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
New memory management policies, new SketchingOperator concept, redesign of DenseDist and SparseDist, lots of docs #108
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…comments and change others.
…e matrix datastructures
…" wrapper functions and the "sketch_sparse" wrapper functions. This functionality is still available through the left_spmm and right_spmm functions, together with the lsksp3 and rsksp3 functions.
…enseDist. And, of course, docs.
…wn_memory for SparseSkOp so that it only attempts to free a constituent array if its pointer is non-null.
… and SparseSkOp. Add a test that SparseSkOp respects ownership semantics.
…ntation of sample_indices_iid_uniform so that it can also populate an array of Rademachers.
…adding an nnz field to SparseSkOp. More tests and probably some cleanup needed. (For example, I think SparseSkOp.nnz!=0 can be used to equivalently signal SparseSkOp.known_filled.)
…olve failures of the address sanitizer tests on linux.
…SparseSkOp, DenseSkOp, and the sparse matrix classes. Make own_memory non-const for sparse matrix classes.
…to add dim_major and dim_minor to SparseDist; will do the same for DenseDist and SketchingDistribution momentarily.
…ndlyOperator struct to replace the old abuses of DenseSkOp that used Axis::Undefined and ScalarDist::BlackBox.
… page. Make a dozen other changes, Im sure.
rileyjmurray
changed the title
WIP: Docs to resolve #107, improved implementation for #84.
New memory management policies, new SketchingOperator concept, redesign of DenseDist and SparseDist, lots of docs
Sep 3, 2024
…pdate DevNotes.md for test_matmul_wrappers.
This was referenced Sep 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch was originally created to expand source and web documentation. However, as I tried to write those docs I found myself questioning many previous design decisions. (It turns out this is a trend.)
Anyway, the PR is now gigantic in scope. I've tried to break down changes into categories.
Changes to source documentation
\mtxA,\mtxB,\mtxX,\mtxS,\mtxx,\mtxy
). Right now those macros render their arguments in bold, but we could easily switch to a different style.\mat(A)[i, j]
in favor of mathematically typeset subscripts like\mat(A)_{ij}
.New features and identifiers
API changes : types
Adopted a consistent policy on semantics of memory ownership. This included making sure all sketching operator and sparse matrix classes have non-const own_memory members.
Added isometry_scale member to SketchingDistribution, DenseDist, and SparseDist.
Added dim_major and dim_minor to DenseDist and SparseDist. Deliberately keeping this out of the SketchingDistribution concept.
RNGState
SparseDist
SparseSkOp
SparseMatrix classes
Important changes that don't affect the public API:
API changes : free functions
fill_sparse
on a user-provided unfilled SparseSkOp. Instead, we make a temporary shallow-copy, fill that copy, and delete it before returning. This is in preparation for when LSKGES/RSKGES only generate submatrices that are needed for the operation at hand, and it follows the same memory-allocation/ownership semantics of dense sketching ([L/R]SK[GE/SP]3).left_spmm
andright_spmm
in the former case andlsksp3
andrsksp3
in the latter case.overwrite_triangle
utility function. I was only using this function with val=0.0.RandBLAS
namespace.print_colmaj
.Templating conventions
sketch_sparse
to avoid nested templating. By nested templating I mean something liketemplate <typename T, typename RNG>
and having an argument of typeDenseSkOp<T,RNG>
, while more "flat" templating would usetemplate <typename DenseSkOp, typename T = DenseSkOp::scalar_t>
and then have arguments of typeT
andDenseSkOp
.SKOP
. They now template asSparseSkOp
(which in a technical sense makes no difference, but does make clear that it's supposed to be aSparseSkOp<T,RNG>
for some parametersT
andRNG
.typename state_t = RNGState<DefaultRNG>
.Web docs
Other
Added a utility script for converting my awkward reStructuredText source code docs of function arguments into doxygen format. I thought the doxygen format might be viable since I tweaked the CSS, but it wasn't. I'm keeping the script in case I want to revisit this later.
Increased the size of allowed error messages in
randblas_error_if_msg
.Added tests for LSKSP3 and RSKSP3.