Tests for interactive API #151
Conversation
alexey-gruzdev
requested review from
aleksandr-mokrov,
dmitryagapov and
igor-davidyuk
|
Jenkins please retry a build |
| # Settings for resizing data | ||
| self.enforce_image_hw = None | ||
| if enforce_image_hw is not None: | ||
| self.enforce_image_hw = tuple(int(size) for size in enforce_image_hw.split(',')) |
There was a problem hiding this comment.
self.enforve_image_hw is it should be tuple?
enforce_image_hw could we use two different variable with type int in order to avoid this transformation?
| if enforce_image_hw is not None: | ||
| self.enforce_image_hw = tuple(int(size) for size in enforce_image_hw.split(',')) | ||
| # Settings for sharding the dataset | ||
| self.rank_worldsize = tuple(int(num) for num in rank_worldsize.split(',')) |
There was a problem hiding this comment.
self.rank_worldsize is it should be tuple?
rank_worldsize could we use two different variable with type int in order to avoid this transformation?
| self.images_names = [ | ||
| img_name | ||
| for img_name in sorted(os.listdir(self.images_path)) | ||
| if len(img_name) > 3 and img_name[-3:] == 'jpg' |
There was a problem hiding this comment.
To check file extension could be used
os.path.splitext(img_name)[1] == ".jpg"or
Path(img_name).suffix == '.mp3'
...ub/interactive_api_director/experiments/pytorch_kvasir_unet/envoy/kvasir_shard_descriptor.py
Show resolved
Hide resolved
| rank_worldsize: 1,90 | ||
| enforce_image_hw: '300,400' |
There was a problem hiding this comment.
could this values divided?
could its have the same type?
There was a problem hiding this comment.
This object reflects 'size' type that by design consists of fixed width and height. For example, the same approach is used for Resize* transforms in torchvision.transforms, where size variable is passed as a tuple or single integer, which means that h=w. In my opinion, these values should be stored together since they are always used together.
There was a problem hiding this comment.
rank_worldsize and enforce_image_hw have diferent types list and string. It looks like it should be more consistent
| self, num_workers=8, batch_size=self.kwargs['train_bs'], sampler=train_sampler | ||
| ) | ||
|
|
||
| def get_valid_loader(self, **kwargs): |
|
|
||
| keeper = Envoy( | ||
| shard_name=shard_name, | ||
| director_uri=director_uri, |
There was a problem hiding this comment.
director_uri was replaced with director_host and director_port
| # 1. target_shape - target shape interface unified across the Federation | ||
|
|
||
| settings: | ||
| sample_shape: ['784'] |
There was a problem hiding this comment.
could '784' be number 784
| if index < len(self.X_train): | ||
| return self.X_train[index], self.y_train[index] | ||
| index -= len(self.X_train) + 1 | ||
| return self.X_test[index], self.y_test[index] |
There was a problem hiding this comment.
It would be greate if we have the same names for similar variables X_train or train_indices and index or indice
|
|
||
| TI = TaskInterface() | ||
| # Task interface currently supports only standalone functions. | ||
| @TI.register_fl_task(model='model', data_loader='train_dataset', \ |
...ub/interactive_api_director/experiments/pytorch_kvasir_unet/envoy/kvasir_shard_descriptor.py
Outdated
Show resolved
Hide resolved
tests/github/interactive_api_director/experiments/pytorch_kvasir_unet/envoy/start_envoy.sh
Outdated
Show resolved
Hide resolved
| self.enforce_image_hw = None | ||
| if enforce_image_hw is not None: | ||
| self.enforce_image_hw = tuple(enforce_image_hw) |
There was a problem hiding this comment.
Could we specify enforce_image_hw: Tuple[int] = None to avoid this check?
There was a problem hiding this comment.
To allow using Tuple here we need to parse YAML collections as tuples. But it requires a specific !!python/tuple tag that is not supported by yaml.safe_load function. All collections are parsed as lists, and they need to be converted to tuples inside ShardDescriptor class since the usage of yaml.safe_load is strictly required.
There was a problem hiding this comment.
Maybe we can convert it to Tuple before we are passing the arguments to constructor?
There was a problem hiding this comment.
We probably could if we created ShardDescriptor instance manually only. But when we spawn envoy via fx envoy start, it reads Shard Descriptor YAML file, calls the constructor of template class, and passes settings as keyword arguments. In this case, the constructor will always receive collections as lists. We can not access intermediate steps between parsing the config file and calling the constructor because it happens inside the library code.
...ub/interactive_api_director/experiments/pytorch_kvasir_unet/envoy/kvasir_shard_descriptor.py
Show resolved
Hide resolved
| tls=False, | ||
| root_certificate='./cert/root_ca.crt', | ||
| private_key='./cert/one.key', | ||
| certificate='./cert/one.crt', |
There was a problem hiding this comment.
root_certificate, private_key, certificate are not required then tls=False
No description provided.