JuliaSmoothOptimizers / PartitionedVectors.jl

Other
8 stars 1 forks source link

Improve code related to CG #22

Closed paraynaud closed 1 year ago

paraynaud commented 1 year ago

Try to solve #21

codecov[bot] commented 1 year ago

Codecov Report

Base: 98.95% // Head: 99.07% // Increases project coverage by +0.12% :tada:

Coverage data is based on head (9636896) compared to base (97dc13f). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #22 +/- ## ========================================== + Coverage 98.95% 99.07% +0.12% ========================================== Files 6 6 Lines 192 217 +25 ========================================== + Hits 190 215 +25 Misses 2 2 ``` | [Impacted Files](https://codecov.io/gh/JuliaSmoothOptimizers/PartitionedVectors.jl/pull/22?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaSmoothOptimizers) | Coverage Δ | | |---|---|---| | [src/krylov.jl](https://codecov.io/gh/JuliaSmoothOptimizers/PartitionedVectors.jl/pull/22/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaSmoothOptimizers#diff-c3JjL2tyeWxvdi5qbA==) | `98.63% <100.00%> (+0.63%)` | :arrow_up: | | [src/struct.jl](https://codecov.io/gh/JuliaSmoothOptimizers/PartitionedVectors.jl/pull/22/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaSmoothOptimizers#diff-c3JjL3N0cnVjdC5qbA==) | `100.00% <0.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaSmoothOptimizers). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaSmoothOptimizers)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

paraynaud commented 1 year ago

@dpo can you review this one?

paraynaud commented 1 year ago

In fact there is no allocations in solve!(::CGSolver). I made a mistake, I performed

@benchmark Krylov.solve!(::CgSolver, ::PartitionedLinearOperator, -pv_gradient)

the allocations came from -pv_gradient which allocates a new PartitionedVector.

pvb = -pv_gradient
@benchmark Krylov.solve!(solver, lo_epm, pvb)
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min … max):  25.700 μs … 228.600 μs  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     26.300 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   29.780 μs ±  13.866 μs  ┊ GC (mean ± σ):  0.00% ± 0.00%
  █▃▂▂▁▁       ▁  ▁▁                                           ▁
  ██████▇▆▆▅▅▅▆██▅██▆▆▅▅▄▄▅▄▄▆▆▇█▇▇▆▇▅▄▅▅▆▄▅▅▅▄▁▄▅▄▁▃▄▁▅▄▄▅▆▆▅ █
  25.7 μs       Histogram: log(frequency) by time       103 μs <
 Memory estimate: 96 bytes, allocs estimate: 4.

for a small example. I changed the partitioned structure of the partitioned linear problem and there was no additional allocations.

dpo commented 1 year ago

ok so is this good to go?

paraynaud commented 1 year ago

ok so is this good to go?

Yes