Add tests for Utilities/sparse_matrix.jl #1369
Conversation
| for term in terms | ||
| colptr[index_map[_variable(term)].value+1] += 1 | ||
| end | ||
| end |
There was a problem hiding this comment.
@blegat what was the motivation to have these _allocate_terms function barriers?
There was a problem hiding this comment.
It dates back to jump-dev/SCS.jl#192 where I benchmarked the two approaches and the function barrier seemed to be faster even if I didn't really get why.
It's maybe not the case anymore in Julia v1.6 or maybe I was mistaken when writing this.
We can revisit later. An easier speedup would be obtained by using @inbounds I guess.
The good thing with MatrixOfConstraints is that we will be able to spend time optimizing these pieces at once in MOI and it will affect all solver's copy_to performance so it might start being worth playing with @inbounds, etc...
There was a problem hiding this comment.
Given the massive amount of new complexity we're introducing, I'd rather that we merged a simplified version first, got it working and debugged, then wrote the various solvers, and only then revisited performance implications with benchmarks.
No bugs. Just some tidying and adding stand-alone tests.