Skip to content
This repository has been archived by the owner on Jun 14, 2020. It is now read-only.

CSRmatrix improvement #96

Closed
wants to merge 13 commits into from
Closed

CSRmatrix improvement #96

wants to merge 13 commits into from

Conversation

ndinsmore
Copy link
Contributor

This makes a slight change to the implementation of CSRMatrix to make it more similar to a proper sparse implementation. This is essentially making row_pointers include the index+1 pointer for the end of the last row.
An interface has also been added to allow access to the properties of CSRMatrix.

The reason for this for this change is to enable speed improvements in Clp

@codecov-io
Copy link

codecov-io commented Mar 12, 2019

Codecov Report

Merging #96 into master will decrease coverage by 3.18%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #96      +/-   ##
==========================================
- Coverage   90.29%   87.11%   -3.19%     
==========================================
  Files          12       12              
  Lines        1339     1350      +11     
==========================================
- Hits         1209     1176      -33     
- Misses        130      174      +44
Impacted Files Coverage Δ
src/constraints/vectorofvariables.jl 95.08% <ø> (-1.64%) ⬇️
src/constraints.jl 92% <ø> (-3.66%) ⬇️
src/mockoptimizer.jl 80.05% <100%> (-3.62%) ⬇️
src/constraints/scalaraffine.jl 93.1% <100%> (-2.68%) ⬇️
src/constraints/vectoraffine.jl 93.68% <100%> (-2.02%) ⬇️
src/solver_interface.jl 25% <0%> (-41.67%) ⬇️
src/objective.jl 78.65% <0%> (-3.38%) ⬇️
src/variables.jl 96.05% <0%> (-2.64%) ⬇️
... and 5 more

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 82246f7...1674535. Read the comment docs.

@ndinsmore
Copy link
Contributor Author

I have updated the pull to replace the CSRMatrix data type with a Adjoint{SparseMatrixCSC}}'

@ndinsmore ndinsmore mentioned this pull request Mar 12, 2019
@ndinsmore
Copy link
Contributor Author

After some review I determined that Transpose was correct rather than Adjoint

@ndinsmore
Copy link
Contributor Author

Similarly is there anything blocking here.

@ndinsmore
Copy link
Contributor Author

@odow
I am trying to figure out if there are blocking issues. This enables a >40x speed increase in clp.

@odow
Copy link
Member

odow commented Apr 2, 2019

Note the failing tests on 0.6. We can probably drop (in a separate PR), and then rebase this on top. Otherwise this is good to go.

@ndinsmore
Copy link
Contributor Author

At the same time should I add testing for 1.1?

@odow
Copy link
Member

odow commented Apr 2, 2019

At the same time should I add testing for 1.1?

Yes, please!

* Update appveyor.yml

remove testing for v0.6 add v1.1

* Update .travis.yml

drop testing for v0.6 add v1.1

* change require to julia 0.7

* This changes the implementation of CSRMatrix to include the additional value at the end of row_pointers that signifies the end of that row

* Remove Depreciation  add sparse conversion

* Small Fix

* Change CSRMatrix to Adjoint{Sparse}}

* Cleanup

* Cleanup comments

* Comment cleanup

* cleanup

* Changed Adjoint to Transpose
@ndinsmore ndinsmore closed this Apr 3, 2019
@ndinsmore ndinsmore deleted the CSRMatrix_improvement branch April 3, 2019 16:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants