Zero source rj by davidrandell84 · Pull Request #14 · sede-open/openMCMC

davidrandell84 · 2026-06-16T12:57:44Z

Description

Extend reversible jump code to handle reversible jump cases with no components.

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Jupyter Notebooks

No changes to existing notebooks

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
Please also list any relevant details for your test configuration

fix_samplers has been extend to handle extra cases including any empty source case
test_prior_recovery has been generalised to compare to a truncated Poisson rather than a full Poisson. This wasn't a problem previously due to the density of the Poisson outside of the range being negligible.
test_parameter has been extended to include cases with size 0 coefficients

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my code
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

… empty case

Signed-off-by: code_reformat <>

…into zero_source_rj

mattj89

Some docstring updates. I don't understand a lot of the code changes (e.g. returning the allocation matrix as a parameter vector from the MixtureParameter cases). Let's discuss next week.

mattj89 · 2026-06-19T14:21:14Z

            parameters that need to be created/removed as part of the dimension change. The default behaviour is to
            sample the necessary additional values from the associated parameter prior distribution. Defaults to None.
-        n_max (int): upper limit on self.param (lower limit is assumed to be 1).
+        n_max (int): upper limit on self.param.


Add: Defaults to None..

Added this for completeness but the fact is pretty much every input to RJ has None as default but is in actual fact required to make it run.

mattj89 · 2026-06-19T14:50:44Z

        """
-        return sparse.diags(diagonals=state[self.param][state[self.allocation]].flatten(), offsets=0, format="csc")
+        if state[self.param].shape[0] == 0:
+            diagonals = state[self.allocation].flatten()


I think that self.allocation is an allocation index (i.e. index vector which determines the allocation of each element of the data to e.g. a kernel). I don't think this should just go on the diagonal of a precision matrix? Totally different to the functionality which is in the other case statement?

have updated to zeros and looked at cases where this is used and it is not used anywhere but the unit tests

mattj89 · 2026-06-19T14:56:31Z


        """
+        if state[self.param].shape[0] == 0:
+            return state[self.allocation]


As with the matrix case (lines 503-508), why would just just set the predictor to be the allocation vector?

have updated to zeros and looked at cases where this is used and it is not used anywhere but the unit tests

Signed-off-by: code_reformat <>

mattj89

I am mostly fine with this- couple of points that we should discuss when I'm back, but then I think we can go ahead and merge.

mattj89 · 2026-08-04T13:59:55Z

+            raise ValueError("Reversible jump MCMC: n_max must be specified.")
+
+        if self.n_max <= self.n_min:
+            raise ValueError("Reversible jump MCMC: n_max must be greater than n_min.")


Should we not check that n_max is strictly greater than n_min? In the case where they're equal, shouldn't be using the RJMCMC?

mattj89 · 2026-08-04T14:06:53Z

+        if log_prop_density.shape[0] > 0:
+            logp_pr_g_cr += np.log(p_birth) + log_prop_density[-1]
+        else:
+            logp_pr_g_cr += np.log(p_birth)


We may have already discussed this, but what situation is this catching? And do we want to resolve it by imposing more requirements on the function calculating log_prop_density instead?

mattj89 · 2026-08-04T14:13:09Z

+        else:
+            diagonals = state[self.param][state[self.allocation]].flatten()
+
+        return sparse.diags(diagonals=diagonals, offsets=0, format="csc")


Think I'm fine with these now- but might be helpful to spend a little time thinking through all potential consequences when I'm back to make sure we're not causing any problems by making this choice.

mattj89 · 2026-08-04T14:21:35Z

    """
    state, n, p = state_tuple
-
+    p = np.maximum(p, 1)


Feel like this needs explanation in the docstring- presumably it's been added to catch the zero-element case, but feels like a fudge. Would it not be better to just assert this case separately? At least explain what should happen in the docstring.

mattj89 · 2026-08-04T14:28:58Z

No issue with the content of this, but as you mentioned, it takes way too long to run.... We should probably think of a solution before merging, sinc we can't realistically expect people to sit for that long when making PRs etc. Maybe just do the [0, 10] case?

davidrandell84 added 3 commits June 11, 2026 14:20

updated reversible jump cases to add min and max state and handle the…

8ff5c99

… empty case

update p_birth death

2e44298

code formatting

4cd6f43

davidrandell84 requested a review from mattj89 June 16, 2026 12:57

code_reformat and others added 4 commits June 16, 2026 12:58

Automatic reformat of code

25b50f9

Signed-off-by: code_reformat <>

minor update

35db004

Merge branch 'zero_source_rj' of https://github.com/sede-open/openMCMC …

2e69ad1

…into zero_source_rj

updated doc string

9cf8d7a

davidrandell84 marked this pull request as ready for review June 17, 2026 07:11

mattj89 requested changes Jun 19, 2026

View reviewed changes

davidrandell84 and others added 5 commits June 22, 2026 13:17

doc string and bug fix

8b41d53

doc string updates

5dde249

Automatic reformat of code

da00929

Signed-off-by: code_reformat <>

updated allocation assignment

e106fc3

Merge remote-tracking branch 'origin/main' into zero_source_rj

fb8185c

mattj89 reviewed Aug 4, 2026

View reviewed changes

Uh oh!

Conversation

davidrandell84 commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Jupyter Notebooks

How Has This Been Tested?

Checklist:

Uh oh!

mattj89 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!

Uh oh!

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!

Uh oh!

Uh oh!

mattj89 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

davidrandell84 commented Jun 16, 2026 •

edited

Loading