Skip to content

Conversation

@aaditgupta21
Copy link
Contributor

#49

@codecov-commenter
Copy link

codecov-commenter commented Jul 13, 2021

Codecov Report

Merging #52 (510d455) into main (6071118) will decrease coverage by 0.09%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #52      +/-   ##
==========================================
- Coverage   18.34%   18.25%   -0.10%     
==========================================
  Files           7        7              
  Lines         398      400       +2     
==========================================
  Hits           73       73              
- Misses        325      327       +2     
Impacted Files Coverage Δ
torchts/utils/graph.py 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6071118...510d455. Read the comment docs.

@klane klane self-requested a review July 13, 2021 15:06
@klane klane added the bug Something isn't working label Jul 15, 2021
@klane klane added this to the Alpha release milestone Jul 15, 2021
Copy link
Collaborator

@klane klane left a comment

Choose a reason for hiding this comment

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

The line as it was originally written was probably fine. We likely just need an if statement inside the undirected if statement that does the following:

  • Check that adj_mx is sparse prior to np.maximum
  • Convert adj_mx to dense only if it is sparse

@klane klane assigned aaditgupta21 and unassigned aaditgupta21 Jul 15, 2021
@klane klane linked an issue Jul 15, 2021 that may be closed by this pull request
@klane
Copy link
Collaborator

klane commented Jul 16, 2021

The line as it was originally written was probably fine. We likely just need an if statement inside the undirected if statement

The function calls normalized_laplacian, which creates a sparse matrix anyway. We can convert sparse matrices to dense at the top of the function regardless of if it is undirected.

klane added 2 commits July 16, 2021 09:15
Prevents error when enforcing symmetry
@klane klane self-requested a review July 16, 2021 16:23
@klane klane changed the title laplacian error fix Fix symmetry enforcement with sparse matrices in Laplacian filter Jul 16, 2021
@klane klane merged commit 39e845f into Rose-STL-Lab:main Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Permit sparse matrices in Laplacian filter

3 participants