pints icon indicating copy to clipboard operation
pints copied to clipboard

Make __call__ check for valid parameter length in various classes

Open ben18785 opened this issue 4 years ago • 1 comments

I just got stumped by the following:

a = pints.toy.RosenbrockError() a.n_parameters() -> 2 (ok) a([1, 3]) -> 400 (ok)

So far so good. Now:

a([1, 3, 4]) -> 400 a([1, 3, 4, 5]) -> 400 a([1, 3, 4, 5, 6, 1, 2, 3, 8, 9]) -> 400

Less good! This is because the toy problem didn't check for an appropriate parameter length.

Some toy problems / toy models do check this: either explicitly or implicitly. Others (like the above) don't.

Since users are going to be inheriting from classes like pints.LogPDF and pints.ForwardModel, I think the above is unsafe. For example, users could mistakenly think that they were fitting a model with (say) 10 parameters, when they were only fitting (say) 5 of them.

My proposal is the following. Make the base classes for ForwardModel, ForwardModelS1, LogPDF, ErrorMeasure implement a parameter length check in their call, __simulate, simulateS1 and so on methods. Then the child classes call super before each of these methods to do the check. This would make the abstract base classes slightly less abstract, I suppose...

This would actually remove repeated code from lots of the toy problems where we do actually do this check.

Thoughts @MichaelClerx and others?

ben18785 avatar Feb 12 '21 01:02 ben18785

Sounds good!

MichaelClerx avatar Feb 12 '21 09:02 MichaelClerx