Support tuples as sample IDs in Bootstrapping #362
Conversation
fuse/eval/metrics/metrics_common.py
Outdated
| @@ -887,6 +887,23 @@ def reset(self) -> None: | |||
| self._metric.reset() | |||
| return super().reset() | |||
|
|
|||
| @staticmethod | |||
| def _convert_tuples(ids: List[Tuple[str, int]]) -> np.ndarray: | |||
There was a problem hiding this comment.
Are you assuming tuple of str and int?
Can we do something more general?
There was a problem hiding this comment.
Thx. The method of convert is supporting tuple of everything of any length, I need to change the interface definition
|
|
||
| ids = np.array(ids, dtype=dtype_tuple) | ||
| ids = [tuple(x) for x in ids] | ||
| return ids |
There was a problem hiding this comment.
Are you returning here a list of tuples or numpy array?
There was a problem hiding this comment.
When applying only line 903 the results numpy array includes tuples but they have a problem of being hashable and I was getting error in the line :
permutation = [original_ids_pos[sample_id] for sample_id in required_ids]
TypeError: unhashable type: 'writeable void-scalar'
The suggestion for the fix I found was to updated their types to tuple that is supported in the list and not in array. If I am trying to convert them after to np.array again the tuples are broken to separate elements and we having 2D array
| @@ -920,7 +941,11 @@ def eval( | |||
| stratum_filter = stratum_id == stratum | |||
| n_stratum = sum(stratum_filter) | |||
| random_sample = rnd.randint(0, n_stratum, size=n_stratum) | |||
| sampled_ids[stratum_filter] = ids[stratum_filter][random_sample] | |||
|
|
|||
| flt_indx = np.where(stratum_filter)[0] | |||
There was a problem hiding this comment.
You needed to move to "for loop implementation" even though you converted it ids to numpy array?
|
@ellabarkan @mosheraboh
I added this hack just to use the code in the meantime Can we add solving this too to this PR and also finalizing it? 
|
|
@mosheraboh I checked and on this branch I get the same exception on line 410, this solution doesn't fix the get method |
|
@mosheraboh Ella and I discussed again, the problem is as follows:
|
The following things were updated to support sample ids presented as tuples: