Conversation
as6520
left a comment
as6520
left a comment
There was a problem hiding this comment.
Looks good for the most part with some minor changes
| from tqdm.auto import trange | ||
| import hashlib | ||
| from collections import OrderedDict | ||
| # from vidaug import augmentors as va |
There was a problem hiding this comment.
If these imports aren't necessary, could you remove them?
| help = "# of groups to generate tests for") | ||
| @click.option("--num_runs", default=1, type=int, | ||
| help = "# of runs for each group") | ||
| @click.option("--num_samples_per_run", default=10, type=int, |
There was a problem hiding this comment.
Should the default be set to 1024, since that is the standard length of the test?
| @click.option("--prob_novel_sample", default=0.5, type=float, | ||
| help="probability of novel sample being introduced " + | ||
| "after 'novelty_timestamp'") | ||
| @click.option("--round_size", default=1, type=int, |
There was a problem hiding this comment.
Should the default be set to 32 since that is the standard round size?
| ), # TODO: confirm that this should be ceil | ||
| "representation": None, # figure out what this should be | ||
| "threshold": 0.5, # TODO: don't hardcode this | ||
| "pre_novelty_batches": math.ceil( |
There was a problem hiding this comment.
I think this should be floor, since post novelty would be in the batch where the red_light_det is
| 'location','relation','relation_type']: | ||
| is_spatial = is_temporal = 0 | ||
| else: | ||
| raise NotImplementedError('Unknown aug_type {}'.format(aug_type)) |
There was a problem hiding this comment.
| raise NotImplementedError('Unknown aug_type {}'.format(aug_type)) | |
| raise NotImplementedError(f'Unknown aug_type {aug_type}') |
|
|
||
| del combine_known_unknown[vid_id] | ||
|
|
||
| assert len(combine_known_unknown) == 0 |
There was a problem hiding this comment.
Is this assertion required since we aren't using this variable beyond this point?
|
|
||
| return return_dict_known, return_dict_unknown | ||
|
|
||
| def gen_test( |
There was a problem hiding this comment.
Should this be gen_tests? Since it looks like it is generating a bunch of tests
2 participants
Incorporates tests for perspective, location, relation, relation_type, as well as continuing to support class as previously done.