teagum / chainsaddiction Goto Github PK

PoisHmm_Delete does not check the input for NULL pointer, which may lead to seg fault.

https://github.com/Teagum/chainsaddiction/blob/772888a414f0e2ea730285064335b480ddf962ae/src/vmath/core.c#L71C1-L71C24

should be *=

• fix typo on line 6
• explicitly including headers is probably not necessary. Check and remove those eventually.

Currently, the Forward/Backward algorithm fails if a state-dependent mean is initialized to a value between 0 and 1. The current workaround introduced in d81e375 is to simply restrict the initialization range to [1, +inf].

• chainsaddiction/src/vmath/vmath.c

  Line 410 in 772888a

  fprintf (stderr, "Maximum of zero sized buffer if not defined.\n");

• chainsaddiction/src/vmath/vmath.c

  Line 431 in 772888a

  fprintf (stderr, "Absolute maximum of zero sized buffer if not defined.\n");

Some of the functions in vmath are not tested. Here is a list:

• poisson_pmf
• poisson_logpmf
• v_poisson_logpmf

Fix these error messages:

• chainsaddiction/src/vmath/vmath.c

  Line 333 in b57995e

  fputs ("Virtual memory exhausted in `m_lse_centroid_rows'.", stderr);

  
  Replace "m_lse_centroid_rows" with "m_lse_centroid_cols"

• chainsaddiction/src/vmath/vmath.c

  Line 410 in b57995e

  fprintf (stderr, "Maximum of zero sized buffer if not defined.\n");

  
  Replace "if" with "is"

• chainsaddiction/src/vmath/vmath.c

  Line 431 in b57995e

  fprintf (stderr, "Absolute maximum of zero sized buffer if not defined.\n");

  
  Replace "if" with "is"

The functions ca_ph_NewParams and ca_ph_NewProbs can potentially cause a memory leak. Each function features three consecutive calls the MA_SCALAR_ZEROS. In case, the second or third of these calls fails due to an allocation error the memory successfully allocated by the previous calls is not correctly deallocated.

This is because MA_SCALAR_ZEROS checks for allocation failures internally. A possible fix is, hence, to directly use malloc.

The random function in PoisHmm_InitRandom generates random samples as expected. For later comparability, however, the variates should be in increasing order.

The HMM has to have a convenient setter, which allows to set/reset the parameters in an PoisParam object.

Should these setters live in the big PoisHmm namespace or should it better have it's own PoisParams namespace, like:

PoisHmm_SetXXX or PoisParams_SetXXX?

The second approach seems more versatile since it would allow to change init and training params. The fist approch would have to be hard coded to set either init or training params.

If the number of states does not match the size of the remaining parameters, the error message is misleading. It reports, that the next header could not be read, which is technically true, but does nevertheless point the user in the wrong direction. Since the actual cause of the problem is the wrong number of states.