Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wrong in TransitionDensity.py #3

Closed
JChonpca opened this issue Dec 30, 2022 · 1 comment
Closed

wrong in TransitionDensity.py #3

JChonpca opened this issue Dec 30, 2022 · 1 comment

Comments

@JChonpca
Copy link

JChonpca commented Dec 30, 2022

I find that if an SDE with term t in the drift or diffusion terms, the fit task will not work anymore.
So I check the code in TransitionDensity.py, and I found that your code confuses t and dt.

Following Code:

class EulerDensity(TransitionDensity):
    def __init__(self, model: Model1D):
        """
        Class which represents the Euler approximation transition density for a model
        :param model: the SDE model, referenced during calls to the transition density
        """
        super().__init__(model=model)

    def __call__(self,
                 x0: Union[float, np.ndarray],
                 xt: Union[float, np.ndarray],
                 t: float) -> Union[float, np.ndarray]:
        """
        The transition density obtained via Euler expansion
        :param x0: float or array, the current value
        :param xt: float or array, the value to transition to  (must be same dimension as x0)
        :param t: float, the time of observing Xt
        :return: probability (same dimension as x0 and xt)
        """
        sig2t = (self._model.diffusion(x0, t) ** 2) * 2 * t
        mut = x0 + self._model.drift(x0, t) * t
        return np.exp(-(xt - mut) ** 2 / sig2t) / np.sqrt(np.pi * sig2t)

In the code above, you pass the dt as t, and then used the dt in the self._model.drift and self._model.diffusion, the wrong likelihood leads to the failure in the fitting task finally. So you need to correct the code by passing the t and dt separately. The correct one may be like the following code.

    def eulerdensity(self,
                 x0: Union[float, np.ndarray],
                 xt: Union[float, np.ndarray],
                 t: float,
                 dt: float) -> Union[float, np.ndarray]:
        """
        The transition density obtained via Euler expansion
        :param x0: float or array, the current value
        :param xt: float or array, the value to transition to  (must be same dimension as x0)
        :param t: float, the time of observing Xt
        :param dt: float, the time setps
        :return: probability (same dimension as x0 and xt)
        """
        sig2t = (self.diffusion(x0, t) ** 2) * 2 * dt
        
        mut = x0 + self.drift(x0, t) * dt
        
        # print(np.exp(-(xt - mut) ** 2 / sig2t) / np.sqrt(np.pi * sig2t))
        
        return np.exp(-(xt - mut) ** 2 / sig2t) / np.sqrt(np.pi * sig2t)

I also suggest that you should check other formulas in the TransitionDensity.py for the same mistake since I only test the EulerDensity part.

Thank you for your effort in this python package, it is really nice as the first package for simulate and estimate SDE systemly with Python language.

@jkirkby3
Copy link
Owner

jkirkby3 commented Jan 1, 2023

Thank you for this great catch! Indeed, the previous code was not suitable for the time-inhomogenous coefficient case. I have refactored the code so that it should support this case, and added some unit tests for the existing (time homogenous) models. I have not yet thoroughly tested time-inhomogenous coefficient case, so please let me know if you see any other issues. See tests\fit\Test_AnalyticalMLE.py for an example usage, specifically:

    Euler_est = AnalyticalMLE(sample=sample, param_bounds=param_bounds, 
                              dt=dt*np.ones(len(sample) - 1),
                              density=EulerDensity(model),
                              t0=np.linspace(0, T-dt, len(sample) - 1),
                              ).estimate_params(guess)

Note how now you have the option to pass in dt as a vector (in this case it's uniform, same dt for each observation) as well as a vector of t0, which is where the coefficients will be evaluated. Please reach out with any questions,

  • Justin

@jkirkby3 jkirkby3 closed this as completed Jan 1, 2023
@jkirkby3 jkirkby3 removed their assignment Jan 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants