Fix bug in zero mean function and add test #358
Conversation
| kernel = kernel.replace_trainable( | ||
| constant=False | ||
| ) # Prevent kernel from modelling non-zero mean |
There was a problem hiding this comment.
Is there any reason we have trainability turned off here?
| likelihood = gpx.Gaussian(num_datapoints=D.n, obs_noise=jnp.array(1e-6)) | ||
| likelihood = likelihood.replace_trainable(obs_noise=False) |
There was a problem hiding this comment.
And trainability turned off here too?
There was a problem hiding this comment.
I just turned off trainability in both cases such that if the Zero mean was still trainable, it would be the only part of the model able to be trained (and so no other aspect of the model (such as the kernel) could be used to model the non-zero mean of the data other than the Zero mean function).
There was a problem hiding this comment.
Nice one @Thomas-Christie. This is looking good.
Good idea to test training loop!
Two further tests that I'd be keen to see are (1) doing a tree_flatten of the Zero - check that that the flatten leaves are an empty list []. (2) Check that you can't initialise a different value for the constant during class initialisation (i.e., Zero(constant=...) returns an Error). These would eliminate bugs, if we did any future refactoring! :)
|
Good idea @daniel-dodd . I have added the two tests you mentioned. |
Type of changes
Checklist
poetry run pre-commit run --all-files --show-diff-on-failurebefore committing.Description
Fixes bug whereby the constant in a zero mean function was able to change from model optimisation.
Issue Number: #330