I am with @britta-wstnr and @LauriParkkonen and after debugging some issues with phantom and simulated data using beamformer code here a list of things that we think should be addressed:
rank default parameter is not consistent between make_lcmv and make_dics
rank='full' in make_lcmv is a misnomer and it's assuming full rank if a noise_cov parameter is passed. We suggest to have an 'auto' mode that would fall back to using the info to infer the data rank.
where inst can be Raw or Epochs or Covariance (info needed if Covariance.
This function would be called early in inverse pipeline so we pass explicitly
and int to all subsequent functions.
Started in #5876.
Remaining items:
[ ] deal with XXX's in rank.py, especially the conflict between np.float64 tolerance for minimum_norm tests and np.float32 necessary for the maxfilter ones in test_rank.py.
[x] add tests for _estimate_rank_by_ratio showing that it's better than estimate_rank
I am with @britta-wstnr and @LauriParkkonen and after debugging some issues with phantom and simulated data using beamformer code here a list of things that we think should be addressed:
rank
default parameter is not consistent betweenmake_lcmv
andmake_dics
rank='full'
in make_lcmv is a misnomer and it's assuming full rank if a noise_cov parameter is passed. We suggest to have an 'auto' mode that would fall back to using theinfo
to infer the data rank.We propose to create a unifying function call
rank = compute_rank(inst, scalings=None, rank='auto', info=None)
where inst can be Raw or Epochs or Covariance (info needed if Covariance. This function would be called early in inverse pipeline so we pass explicitly and int to all subsequent functions.
Started in #5876.
Remaining items:
rank.py
, especially the conflict between np.float64 tolerance for minimum_norm tests and np.float32 necessary for the maxfilter ones in test_rank.py.