Simplify ParticleContainer

[devmotion]

This PR simplifies the type parameters of ParticleContainer, and removes the conditional field which by definition was always nothing.

Moreover, instead of appending newly allocated arrays, push! now resizes the existing arrays and sets the new values, which is faster:

using BenchmarkTools

function f(x, n)
    y = zeros(eltype(x), n)
    append!(x, y)
    nothing
end

function g(x, n)
    m = length(x)
    mpn = m + n
    resize!(x, mpn)
    @inbounds for i in (m + 1):mpn
        x[i] = zero(eltype(x))
    end
    nothing
end

a = rand(1_000)

@btime f($(copy(a)), 100) # 339.092 ns (1 allocation: 896 bytes)
@btime g($(copy(a)), 100) # 263.981 ns (0 allocations: 0 bytes)

The empty! function now defines an array of particles whose element type is based on the previously used array instead of always using the abstract element type Particle.

The copy! function of ParticleContainer creates a set of copied particles by list comprehension instead of pushing them individually to an initially empty array.

Moreover, the definition of Trace is simplified by removing the completely uninitialized inner constructor and reordering the fields (making it easier to create new instances without specifying a task).

[cpfiffer]

I'm really glad to see some of this stuff updated. The particle stuff has always been a little rickety.

Anyone know why tests aren't running for this PR?

[mohamed82008]

I think there is a problem with tests on all our PRs.

[yebai]

The PR looks correct to me. It might be helpful to run some tests on the PR before merging, say after the Zygote related dependency mess is settled.

[yebai]

@devmotion The CI is now fixed on the master branch. Can you try to rebase master into this PR? Once all tests pass, I'm happy to merge it.

[devmotion]

I rebased on master, but as observed above tests aren't running. Skimming through other PRs it seems that tests don't run for PRs from forks?

[yebai]

Yep; seems that's true. I don't know how to activate CI on forks. Since you're a repo developer now, do you want to create a new branch inside this repo?

[devmotion]

Sure, I can do that. I just noticed that apparently the actions are run on my fork but not on the main repo: https://github.com/devmotion/Turing.jl/actions

[yebai]

Sure, I can do that. I just noticed that apparently the actions are run on my fork but not on the main repo: https://github.com/devmotion/Turing.jl/actions

That is equivalent - happy to merge once the actions based CI passes.

[devmotion]

It's not completely clear to me from the documentation, but maybe the issue for PRs could be solved by replacing lines such as

Turing.jl/.github/workflows/DynamicHMC.yml

Line 3 in 87a3db9

on: [push] # [push, pull_request]

with on: [push, pull_request]. To run the Travis CIs for all PRs, one should probably enable Build pushed pull requests in the settings of Travis.

[yebai]

Thanks, Travis CI is now turned on. I’m not sure about the actions based CI though, cause turning both push and pull request on causes duplicate builds in the past.

[devmotion]

I guess we'll face the same issue with Travis CI and end up with duplicate tests for forks from the main repo. However, I think that can be avoided by adding

branches:
  only:
    - master

to .travis.yml (at least that's what Google told me 😄).

[devmotion]

Apparently, in a similar way Github Actions can be filtered: https://help.github.com/en/actions/automating-your-workflow-with-github-actions/configuring-a-workflow#filtering-for-specific-branches-tags-and-paths

[yebai]

That looks quite promising. Would you like to give it a try in this PR? It seems I can't push to this PR easily.