harrispopgen / mushi

[mu]tation [s]pectrum [h]istory [i]nference
https://harrispopgen.github.io/mushi/
MIT License
24 stars 5 forks source link

trend penalty piece of cost functions using incorrect difference order #74

Closed wsdewitt closed 2 years ago

wsdewitt commented 3 years ago

A k-th order trend filter requires a penalty on (k+1)-th order differences. For example, a 0th order trend filter uses a penalty on first differences. It appears that in two places only k-th order differences are used for computing the cost. The impact of this bug is limited to the tolerance for outer optimization convergence, and does not impact optimizations done for a specified number of iterates, and does not impact the inner trend filtering optimization (prox operator).

https://github.com/harrispopgen/mushi/blob/f962fe89ba3914837ff62abe125b85fddf877ea4/mushi/ksfs.py#L443-L446

https://github.com/harrispopgen/mushi/blob/f962fe89ba3914837ff62abe125b85fddf877ea4/mushi/ksfs.py#L603-L607

I suggest writing a shared function for computing the trend cost, and a unit test to validate a by-hand example.