Add ESS resampler to avoid resampling every iteration by jihodori · Pull Request #67 · JuliaPOMDP/ParticleFilters.jl

jihodori · 2024-04-26T02:32:20Z

Adding ESS rampler and setting the default threshold to 0.5.

Tested with runtests.jl

Test Summary: | Pass  Total  Time
implemented   |    2      2  0.0s
WARNING: redefinition of constant Main.A. This may fail, cause incorrect answers, or produce other errors.
WARNING: redefinition of constant Main.B. This may fail, cause incorrect answers, or produce other errors.
WARNING: redefinition of constant Main.W. This may fail, cause incorrect answers, or produce other errors.
WARNING: redefinition of constant Main.V. This may fail, cause incorrect answers, or produce other errors.
....................................................................................................Test Summary: |Time
example       | None  0.4s
Test Summary:   | Pass  Total  Time
domain_specific |    7      7  0.2s
Test Summary: | Pass  Total  Time
beliefs       |   24     24  0.1s
Test Summary: | Pass  Total  Time
infer         |    2      2  0.0s
Test Summary:  | Pass  Total  Time
pomdp terminal |    1      1  0.4s
Test Summary: | Pass  Total  Time
alpha         |    2      2  0.4s

zsunberg

Thanks for the PR! I think we need to make some bigger changes for this to work correctly. See #68.

…tests

jihodori · 2024-06-21T02:32:14Z

+    if (calculate_ess(wm) < up.resampling_threshold)
+        resampled_particle_collection = resample(
+            up.resampler,
+            WeightedParticleBelief(pm, wm, sum(wm), nothing),
+            up.predict_model,
+            up.reweight_model,
+            b, a, o,
+            up.rng)
+		num_particles = n_particles(resampled_particle_collection)
+        return WeightedParticleBelief(resampled_particle_collection.particles, fill(1.0 / num_particles, num_particles))
+    end


actually, if you resample before reweight!, the update function does not work as expected

now resampling at the end

I don't quite understand this decision. It seems much better to resample before prediction and I don't understand why it didn't work.

I don't quite understand this decision. It seems much better to resample before prediction and I don't understand why it didn't work.

I handled the edge case (first iteration) by checking wm is empty. The resample function should not be called when the update function is called for the first time

zsunberg

I think this looks pretty good, but please address my inline comment about resampling before predicting.

(also, I am seeing many things that I don't like about the code I wrote - it seems like it might be time for a major revamp after this is merged 😄 )

zsunberg · 2024-06-28T23:14:48Z

+    if (calculate_ess(wm) < up.resampling_threshold)
+        resampled_particle_collection = resample(
+            up.resampler,
+            WeightedParticleBelief(pm, wm, sum(wm), nothing),
+            up.predict_model,
+            up.reweight_model,
+            b, a, o,
+            up.rng)
+		num_particles = n_particles(resampled_particle_collection)
+        return WeightedParticleBelief(resampled_particle_collection.particles, fill(1.0 / num_particles, num_particles))
+    end


I don't quite understand this decision. It seems much better to resample before prediction and I don't understand why it didn't work.

zsunberg

This looks really good. I just have one important comment about checking the weights, and a few minor code comments.

zsunberg · 2024-07-11T04:31:24Z

    pf = BootstrapFilter(pomdp, 100)
    bp = update(pf, initialize_belief(pf, Categorical([0.5, 0.5])), -1, 1.0)
-    @test all(particles(bp) .== 1)
+    @test abs(mean(bp) - 1.0) < 1e-5


Wouldn't it be better to test that all particles with nonzero weight are 1? or pdf(bp, 1.0) is approximately 1?

Wouldn't it be better to test that all particles with nonzero weight are 1

I agree with this. Addressed this in the newer commit

zsunberg · 2024-07-11T04:47:49Z

+function update(up::BasicParticleFilter, b::AbstractParticleBelief, a, o)
    pm = up._particle_memory
    wm = up._weight_memory
+    if (!isempty(wm) && calculate_ess(wm) < up.resampling_threshold)


Why do we need to check if wm is empty? Shouldn't calculate_ess prevent resampling on the first step? Also, the same updater might be re-used for multiple simulations, so this is not a reliable way to check.

(In general, to write maintainable code, it is not good to check implicit side effects (e.g. wm will probably be empty at the beginning, but that is not guaranteed) it is better to explicitly check whether it is the first step 😄 but, by design, that is not possible)

Sorry it took me some time to figure out why I had this conditional statement. I realized I was testing this after calling the resize! function with an empty list (the initial case), which causes wm to have actual small weights such that calculate_ess is always lower than the resampling_threshold.

Now calculate_ess is called before the resize! function and prevents resampling on the first step as it returns Inf

zsunberg · 2024-07-11T04:49:19Z

+ESS is divided by `num_particles` to make it easier to get a uniform threshold (scale between 0 and 1) for resampling across different particle filters.
+
+M. S. Arulampalam, S. Maskell, N. Gordon and T. Clapp, "A tutorial on particle filters for online nonlinear/non-Gaussian Bayesian tracking," in IEEE Transactions on Signal Processing, vol. 50, no. 2, pp. 174-188, Feb. 2002, doi: 10.1109/78.978374.
+keywords: {Tutorial;Particle filters;Nonlinear dynamical systems;Costs;Signal processing;Bayesian methods;Particle tracking;Kalman filters;Filtering;Monte Carlo methods},


Delete these keywords?

zsunberg · 2024-07-11T04:50:52Z

+M. S. Arulampalam, S. Maskell, N. Gordon and T. Clapp, "A tutorial on particle filters for online nonlinear/non-Gaussian Bayesian tracking," in IEEE Transactions on Signal Processing, vol. 50, no. 2, pp. 174-188, Feb. 2002, doi: 10.1109/78.978374.
+keywords: {Tutorial;Particle filters;Nonlinear dynamical systems;Costs;Signal processing;Bayesian methods;Particle tracking;Kalman filters;Filtering;Monte Carlo methods},
+"""
+function calculate_ess(weights)


Suggested change

function calculate_ess(weights)

function normalized_ess(weights)

"calculate" does not really add anything to the name of this function. Also, If someone just said "ESS", does it mean a number between 0 and 1 or an actual number of particles? If ESS means a number of particles, I think we should call this function normalized_ess.

changed the name to normalized_ess

add ess resampler to avoid resampling every iteration

e86fb8f

jihodori changed the title ~~add ess resampler to avoid resampling every iteration~~ Add ESS resampler to avoid resampling every iteration Apr 26, 2024

jihodori mentioned this pull request Apr 26, 2024

Add resampling criteria #24

Open

zsunberg mentioned this pull request Jun 12, 2024

Improved bootstrap filter #68

Open

zsunberg requested changes Jun 12, 2024

View reviewed changes

Comment thread src/basic.jl Outdated

Comment thread src/basic.jl Outdated

jihodori and others added 6 commits June 18, 2024 00:21

change update function to return WeightedParticleBelief and fix unit …

138a40a

…tests

update indentation

8ab300e

fix indentation

71143b6

Merge branch 'JuliaPOMDP:master' into effective-sample-size

45cdce9

fix indentation

f0052f0

fix unnecessary changes

36fc09d

jihodori commented Jun 21, 2024

View reviewed changes

jihodori added 2 commits June 20, 2024 20:39

resample at the end

d38895c

delete unnecessary code

74d090d

zsunberg requested changes Jun 28, 2024

View reviewed changes

jihodori added 2 commits July 3, 2024 17:06

resample at the beginning of the update function

08d8140

fix indentation

3839144

zsunberg requested changes Jul 11, 2024

View reviewed changes

remove checking wm is empty and update the test code

00eb873

	function calculate_ess(weights)
	function normalized_ess(weights)

Conversation

jihodori commented Apr 26, 2024

Uh oh!

zsunberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zsunberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zsunberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants