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

document invariants for nnz_expanded_ or remove it entirely #166

Closed
superwhiskers opened this issue Jun 19, 2024 · 1 comment · Fixed by #184
Closed

document invariants for nnz_expanded_ or remove it entirely #166

superwhiskers opened this issue Jun 19, 2024 · 1 comment · Fixed by #184
Assignees
Labels
documentation Improvements or additions to documentation question Further information is requested

Comments

@superwhiskers
Copy link
Collaborator

according to @pelesh, there is no hard requirement that the value stored in the nnz_expanded_ field on the Sparse class be correct. the field should either be removed entirely (as i'm not really sure what it would be used for if it can't be relied on) or have this invariant documented so that it does not lead to confusion or errors in the future

@superwhiskers superwhiskers added the documentation Improvements or additions to documentation label Jun 19, 2024
@superwhiskers superwhiskers changed the title document invariants for nnz_expanded or remove it entirely document invariants for nnz_expanded_ or remove it entirely Jun 19, 2024
@pelesh
Copy link
Collaborator

pelesh commented Jun 20, 2024

Thanks for reporting this issue, @superwhiskers.

The nnz_expanded_ refers to the number of non zeros when you expand a symmetric matrix to a general sparse matrix format. There is no way of knowing nnz_expanded_ upon instantiation of a sparse symmetric matrix in upper/lower triangular form, so it should be set to zero and stay zero for al long as flag is_expanded_ is set to false. The nnz_expanded_ needs to be calculated at the time when a function to expand the matrix into a general form is called. The expand function should have the responsibility to set the new number of nonzeros and flip the flag is_expanded_ to true.

My preference would be to remove nnz_expanded_ as a member variable altogether because it is used only as a temporary storage until nnz_ is updated.

CC @kswirydo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants