GWeindel / hmp

Repository for the hmp python package
BSD 3-Clause "New" or "Revised" License
35 stars 8 forks source link

Further work on condition specific codes #81

Closed GWeindel closed 1 year ago

GWeindel commented 1 year ago

The new PR #80 introduces a new way of fitting HMP across conditions while sharing (or not) some parameters.

Work left before definitive integration to a new version stored on pip:

jelmerborst commented 1 year ago

EM and estim_probs have been merged.

fit_single is harder to merge, as it can handle so many different inputs. Better approach is probably to add functions for some of the parts that are shared by fit_single and fit_single_cond

Another todo is to merge and/or simplify the plotting code.

GWeindel commented 1 year ago

I ran into this error while executing tutorial 3:

---------------------------------------------------------------------------
RemoteTraceback                           Traceback (most recent call last)
RemoteTraceback: 
"""
Traceback (most recent call last):
  File "/home/gweindel/anaconda3/envs/hmp/lib/python3.11/multiprocessing/pool.py", line 125, in worker
    result = (True, func(*args, **kwds))
                    ^^^^^^^^^^^^^^^^^^^
  File "/home/gweindel/anaconda3/envs/hmp/lib/python3.11/multiprocessing/pool.py", line 51, in starmapstar
    return list(itertools.starmap(args[0], args[1]))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/gweindel/owncloud/projects/RUGUU/hsmm-mvpy/src/hsmm_mvpy/utils.py", line 644, in LOOCV
    likelihood = model_left_out.estim_probs(fit.magnitudes.dropna('event').values, fit.parameters, n_events,True)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/gweindel/owncloud/projects/RUGUU/hsmm-mvpy/src/hsmm_mvpy/models.py", line 807, in estim_probs
    if len(subset_epochs) == self.n_trials: #boolean indices
       ^^^^^^^^^^^^^^^^^^
TypeError: object of type 'bool' has no len()
"""
GWeindel commented 1 year ago

I guess this is because of the following line in utils.py (L.644):

    likelihood = model_left_out.estim_probs(fit.magnitudes.dropna('event').values, fit.parameters, n_events,True)

So I just removed the n_events as you dropped the need for that

likelihood = model_left_out.estim_probs(fit.magnitudes.dropna('event').values, fit.parameters ,True)
GWeindel commented 1 year ago

Ahh no it's the new subset_epochs=None, fixed in next commit

jelmerborst commented 1 year ago

good catch!

jelmerborst commented 1 year ago

all finished with #102