Closed AntoninKns closed 2 years ago
Merging #46 (e4978ac) into main (4b6d226) will increase coverage by
0.77%
. The diff coverage is97.50%
.
@@ Coverage Diff @@
## main #46 +/- ##
==========================================
+ Coverage 89.61% 90.38% +0.77%
==========================================
Files 5 5
Lines 308 312 +4
==========================================
+ Hits 276 282 +6
+ Misses 32 30 -2
Impacted Files | Coverage Δ | |
---|---|---|
src/JacobianByHand.jl | 96.77% <90.90%> (+0.16%) |
:arrow_up: |
src/BundleAdjustmentNLSFunctions.jl | 93.33% <100.00%> (+1.73%) |
:arrow_up: |
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 4b6d226...e4978ac. Read the comment docs.
It seems there are some problems with NLPModels counters. I realised a lot of them allocate memory. After trying to go more in depth I think I found the reason why. A lot of these functions call fieldnames(Counters)
like here. And this call create a tuple of the fieldnames of the counters. After examination it seems that removing this call avoids memory allocations in the functions. I will create an issue on this and try to solve it. In the meantime I have added unit tests to check memory allocations on the residual function, jac_structure and jac_coord functions that I will commit soon.
I added tests for Julia version above 1.6 because memory allocation was different for Julia 1.3. Allocations should be 0 with the right NLPModels version but I thought it would be better to allow the use of the package even if the functions allocate some memory. If everything is clear we can merge and update the package version to allow the use of non allocating residual and jacobian.
Here are the results of benchmarks for Julia 1.7.3
julia> @benchmark residual!($model, $model.meta.x0, $r)
BenchmarkTools.Trial: 1011 samples with 1 evaluation.
Range (min … max): 4.854 ms … 7.829 ms ┊ GC (min … max): 0.00% … 0.00%
Time (median): 4.898 ms ┊ GC (median): 0.00%
Time (mean ± σ): 4.938 ms ± 190.290 μs ┊ GC (mean ± σ): 0.00% ± 0.00%
▅██▆▅▄▃▂
████████▇▇▇▅▄▄▅▅▁▆▄▅▅▁▄▁▁▄▁▄▁▁▄▁▄▁▄▁▁▅▁▁▁▄▄▅▄▁▁▄▁▁▁▁▄▅▄▁▁▁▄ █
4.85 ms Histogram: log(frequency) by time 5.88 ms <
Memory estimate: 0 bytes, allocs estimate: 0.
For jacobian functions with the pull request with corrections to allocations, it should be down to 0.
julia> @benchmark jac_structure!($model, $model.rows, $model.cols)
BenchmarkTools.Trial: 2001 samples with 1 evaluation.
Range (min … max): 2.408 ms … 4.678 ms ┊ GC (min … max): 0.00% … 0.00%
Time (median): 2.458 ms ┊ GC (median): 0.00%
Time (mean ± σ): 2.488 ms ± 142.112 μs ┊ GC (mean ± σ): 0.00% ± 0.00%
▁▂▇█▃▂
██████▇▆▅▄▄▃▃▃▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▂▁▂▂▂▂▂▁▂▂▂▂▁▂▁▂▂▁▂▂▂▂▁▂▂▁▁▁▂▂ ▃
2.41 ms Histogram: frequency by time 3.15 ms <
Memory estimate: 16 bytes, allocs estimate: 1.
julia> @benchmark jac_coord!($model, $model.meta.x0, $model.vals)
BenchmarkTools.Trial: 239 samples with 1 evaluation.
Range (min … max): 20.653 ms … 31.367 ms ┊ GC (min … max): 0.00% … 0.00%
Time (median): 20.800 ms ┊ GC (median): 0.00%
Time (mean ± σ): 20.968 ms ± 796.776 μs ┊ GC (mean ± σ): 0.00% ± 0.00%
▄▅█▆▃▁▁ ▁
███████▆██▆▄▄▁▆▆▆▁▄▄▁▁▁▁▄▁▁▄▄▄▄▄▁▁▁▄▁▁▁▁▁▁▄▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▄ ▆
20.7 ms Histogram: log(frequency) by time 23.8 ms <
Memory estimate: 16 bytes, allocs estimate: 1.
Even with the NLPModels update there are some memory allocations hidden in the code. I am currently working on solving them.
After some different tests and researches, it seems that only the @allocated
macro tracks memory but neither the use of --track-allocation=all
nor @benchmark
seems to detect it as we can see below :
julia> @benchmark residual!($model, $model.meta.x0, $r)
BenchmarkTools.Trial: 1032 samples with 1 evaluation.
Range (min … max): 4.670 ms … 6.940 ms ┊ GC (min … max): 0.00% … 0.00%
Time (median): 4.823 ms ┊ GC (median): 0.00%
Time (mean ± σ): 4.843 ms ± 184.145 μs ┊ GC (mean ± σ): 0.00% ± 0.00%
▇ ▁▁▁▆█▃
▄▄▇█▇█▇██████▆▆▄▄▃▃▃▂▂▃▂▂▂▂▂▃▂▂▂▂▂▂▂▂▁▂▂▁▂▂▂▂▁▁▂▁▁▁▁▂▂▁▂▁▂▂ ▃
4.67 ms Histogram: frequency by time 5.57 ms <
Memory estimate: 0 bytes, allocs estimate: 0.
julia> @benchmark jac_structure!($model, $model.rows, $model.cols)
BenchmarkTools.Trial: 2052 samples with 1 evaluation.
Range (min … max): 2.340 ms … 5.022 ms ┊ GC (min … max): 0.00% … 0.00%
Time (median): 2.379 ms ┊ GC (median): 0.00%
Time (mean ± σ): 2.431 ms ± 177.761 μs ┊ GC (mean ± σ): 0.00% ± 0.00%
▅██▆▄▄▅▄▄▃▁▂▂▁▁▁ ▁
███████████████████▇▇▇▇█▆▆▇▆▅▆▆▆▅▄▄▄▅▄▁▁▄▁▄▄▄▄▄▄▄▄▁▁▄▄▁▁▄▅▄ █
2.34 ms Histogram: log(frequency) by time 3.11 ms <
Memory estimate: 0 bytes, allocs estimate: 0.
julia> @benchmark jac_coord!($model, $model.meta.x0, $model.vals)
BenchmarkTools.Trial: 224 samples with 1 evaluation.
Range (min … max): 20.780 ms … 44.262 ms ┊ GC (min … max): 0.00% … 0.00%
Time (median): 21.619 ms ┊ GC (median): 0.00%
Time (mean ± σ): 22.376 ms ± 2.745 ms ┊ GC (mean ± σ): 0.00% ± 0.00%
▇▆█▆▅▅▄▃▂
███████████▆▇▅▅▁▁▆▆▅▆▁▁▁▁▅▅▁▁▁▁▁▁▁▁▅▅▁▅▅▁▁▁▁▁▅▅▁▁▁▅▁▁▁▁▁▁▁▅ ▆
20.8 ms Histogram: log(frequency) by time 34.5 ms <
Memory estimate: 0 bytes, allocs estimate: 0.
It seems that it is due to some environment issues because using the @allocated
macro inside the residual!
function for example does not detect any memory allocation. I am trying to create a better environment to use the allocated macro to avoid memory leaks like these and I will push it.
Using wrappers around the function as pointed by this forum thread solved the problem.
Tests on problem-16-22106-pre :
julia> @benchmark residual!($model, $model.meta.x0, $r)
BenchmarkTools.Trial: 403 samples with 1 evaluation.
Range (min … max): 11.730 ms … 14.030 ms ┊ GC (min … max): 0.00% … 0.00%
Time (median): 12.309 ms ┊ GC (median): 0.00%
Time (mean ± σ): 12.403 ms ± 353.824 μs ┊ GC (mean ± σ): 0.00% ± 0.00%
▂█▄█▆▄▄ ▂
▂▁▃▃▄▄▄▅▄▆▄▅▇███████████▅▆▄▃▃▄█▄▅▄▄▄▃▅▄▄▅▄▃▃▅▃▂▂▃▃▁▂▃▂▁▁▂▁▁▂ ▄
11.7 ms Histogram: frequency by time 13.5 ms <
Memory estimate: 0 bytes, allocs estimate: 0.
julia> @benchmark jac_structure!($model, $model.rows, $model.cols)
BenchmarkTools.Trial: 762 samples with 1 evaluation.
Range (min … max): 6.201 ms … 8.790 ms ┊ GC (min … max): 0.00% … 0.00%
Time (median): 6.474 ms ┊ GC (median): 0.00%
Time (mean ± σ): 6.554 ms ± 307.828 μs ┊ GC (mean ± σ): 0.00% ± 0.00%
▂▅█▃▃▇▅█▂
▃▄▃██████████▆▅▇▄▆▅▃▄▃▃▄▃▃▃▂▃▂▁▃▃▃▂▂▃▂▂▃▂▂▁▂▂▂▁▂▂▁▂▁▂▁▁▁▂▁▂ ▃
6.2 ms Histogram: frequency by time 7.88 ms <
Memory estimate: 0 bytes, allocs estimate: 0.
julia> @benchmark jac_coord!($model, $model.meta.x0, $model.vals)
BenchmarkTools.Trial: 92 samples with 1 evaluation.
Range (min … max): 53.715 ms … 56.856 ms ┊ GC (min … max): 0.00% … 0.00%
Time (median): 54.716 ms ┊ GC (median): 0.00%
Time (mean ± σ): 54.775 ms ± 504.162 μs ┊ GC (mean ± σ): 0.00% ± 0.00%
▂▂▂ ▁█▇ ▂ ▁ ▁
▅▃▁▁▁▁▆▁▁▅▅▃▁▅███████▅█▅▅▅█▁▃█▃▅▁▁▁▁▃▁▃▁▁▁▁▁▁▁▁▁▁▁▁▃▁▁▁▁▁▁▁▃ ▁
53.7 ms Histogram: frequency by time 56.8 ms <
Memory estimate: 0 bytes, allocs estimate: 0.
Even with a bigger problem like problem-1158-802917-pre it still works :
julia> @benchmark residual!($model, $model.meta.x0, $r)
BenchmarkTools.Trial: 8 samples with 1 evaluation.
Range (min … max): 636.564 ms … 754.905 ms ┊ GC (min … max): 0.00% … 0.00%
Time (median): 649.672 ms ┊ GC (median): 0.00%
Time (mean ± σ): 673.749 ms ± 48.464 ms ┊ GC (mean ± σ): 0.00% ± 0.00%
█ ██ ██ █ █ █
█▁▁██▁██▁▁▁▁▁█▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁█▁▁█ ▁
637 ms Histogram: frequency by time 755 ms <
Memory estimate: 0 bytes, allocs estimate: 0.
julia> @benchmark jac_structure!($model, $model.rows, $model.cols)
BenchmarkTools.Trial: 16 samples with 1 evaluation.
Range (min … max): 311.779 ms … 363.857 ms ┊ GC (min … max): 0.00% … 0.00%
Time (median): 322.623 ms ┊ GC (median): 0.00%
Time (mean ± σ): 326.465 ms ± 14.787 ms ┊ GC (mean ± σ): 0.00% ± 0.00%
▁ █ ▁ █▁ ▁▁█ ▁▁ ▁ ▁ ▁
█▁▁▁█▁▁█▁██▁███▁▁██▁█▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁█▁▁█ ▁
312 ms Histogram: frequency by time 364 ms <
Memory estimate: 0 bytes, allocs estimate: 0.
julia> @benchmark jac_coord!($model, $model.meta.x0, $model.vals)
BenchmarkTools.Trial: 2 samples with 1 evaluation.
Range (min … max): 2.757 s … 2.761 s ┊ GC (min … max): 0.00% … 0.00%
Time (median): 2.759 s ┊ GC (median): 0.00%
Time (mean ± σ): 2.759 s ± 2.803 ms ┊ GC (mean ± σ): 0.00% ± 0.00%
█ █
█▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁█ ▁
2.76 s Histogram: frequency by time 2.76 s <
Memory estimate: 0 bytes, allocs estimate: 0.
Excellent, thank you!
Removed all allocations on
jac_coord!
. When I tested locally there still seemed to have allocations in theincrement!
function even though I updated NLPModels to 0.18.5. Apart from that there are no more allocations.