Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use multi-dimensional arrays to represent states #55

Merged
merged 2 commits into from
May 19, 2020
Merged

Conversation

giordano
Copy link
Member

@giordano giordano commented May 18, 2020

This implements one of the possible improvements suggested in #53. However, I don't see any actual improvement in memory allocations compared to what was reported there:

julia> tdac(TDAC.tdac_params(;nprt = 1, n_time_step = 1, n_integration_step = 251, time_step = 251.0, nobs = 4, enable_timers = true));
 ────────────────────────────────────────────────────────────────────────────────
                                         Time                   Allocations      
                                 ──────────────────────   ───────────────────────
        Tot / % measured:             465ms / 100%             667MiB / 100%     

 Section                 ncalls     time   %tot     avg     alloc   %tot      avg
 ────────────────────────────────────────────────────────────────────────────────
 Particle State Update        1    183ms  39.4%   183ms    306MiB  46.0%   306MiB
 True State Update            1    167ms  35.9%   167ms    306MiB  46.0%   306MiB
 Initialization               1   92.7ms  20.0%  92.7ms   43.6MiB  6.54%  43.6MiB
 Process Noise                1   20.4ms  4.40%  20.4ms   8.55MiB  1.28%  8.55MiB
 Particle Variance            1    668μs  0.14%   668μs   1.83MiB  0.27%  1.83MiB
 Resample                     1    265μs  0.06%   265μs     96.0B  0.00%    96.0B
 Particle Mean                1    238μs  0.05%   238μs     0.00B  0.00%    0.00B
 State Copy                   1   77.6μs  0.02%  77.6μs     32.0B  0.00%    32.0B
 Weights                      1   14.0μs  0.00%  14.0μs   1.03KiB  0.00%  1.03KiB
 Observations                 2   11.3μs  0.00%  5.66μs      224B  0.00%     112B
 Observation Noise            1    827ns  0.00%   827ns     48.0B  0.00%    48.0B
 ────────────────────────────────────────────────────────────────────────────────

Nevertheless, I think that having the states represented with multi-dimensional arrays simplifies the code in many places as it allows us to avoid doing complex index arithmetic, we don't need dim_grid and dim_state at all, and we don't need to flatten arrays in a few places.

@giordano giordano marked this pull request as ready for review May 19, 2020 09:41
@giordano giordano requested a review from tkoskela May 19, 2020 09:41
src/TDAC.jl Outdated
Comment on lines 420 to 416
ds,dtype = d_create(subgroup, dataset_name, @view(state[1:params.dim_grid]))
ds[1:params.dim_grid] = @view(state[1:params.dim_grid])
ds,dtype = d_create(subgroup, dataset_name, state)
ds[:, :, 1] = @view(state[:, :, 1])
Copy link
Member

Choose a reason for hiding this comment

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

The d_create function in HDF5.jl was so weird that I have to check what this actually does. It looks like it may be creating a dataset with the size of all the variables, and we only want one.

@giordano giordano merged commit 3db6c1e into master May 19, 2020
@giordano giordano deleted the mg/state-arrays branch May 19, 2020 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants