-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Бенчмаркинг PaddlePaddle] Реализация пайплайна для PaddlePaddle #508
[Бенчмаркинг PaddlePaddle] Реализация пайплайна для PaddlePaddle #508
Conversation
for name in outputs: | ||
output_tensor = predictor.get_output_handle(name) | ||
output_data = output_tensor.copy_to_cpu() | ||
result.append(output_data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь не должен создаваться словарь, в котором ключами являются названия выходов, с значениями - соответствующие тензора?
if (output_names is None) or (len(result) != len(output_names)): | ||
raise ValueError('The number of output tensors does not match the number of corresponding output names') | ||
if task == 'classification': | ||
return {output_names[i]: result[i] for i in range(len(result))} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Согласно описанному выше комментарию, если сделать так, как там написано, то здесь можно просто вернуть результат.
config = paddle_infer.Config(args.model_path, args.params_path) | ||
config.enable_memory_optim() | ||
if args.device == 'GPU': | ||
config.enable_use_gpu(1000, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Что за магическая константа 1000?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
размер выделенной помяти в gpu, 1000 - позмер из всех найденых мной примеров
parser.add_argument('-t', '--task', | ||
help='Output processing method. Default: without postprocess', | ||
choices=['segmentation', 'classification', 'detection', 'yolo_tiny_voc', 'yolo_v2_coco', | ||
'yolo_v2_tiny_coco', 'yolo_v3_tf', 'mask-rcnn'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь надо оставить только те задачи, до которых поддерживается валидация выходов.
default=None, | ||
type=str, | ||
nargs=1, | ||
dest='delegate_ext') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Этот параметр явно не относится к PaddlePaddle, неудачный копи-паст :)
default=None, | ||
type=str, | ||
nargs=1, | ||
dest='delegate_options') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
И этот тоже :) Посмотрите список параметров на предмет их необходимости в текущей реализации вывода.
dest='labels') | ||
parser.add_argument('--report_path', | ||
type=Path, | ||
default=Path(__file__).parent / 'tflite_inference_report.json', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tflite_inference_report? Может все-таки paddle_inference_report?
time_infer, _ = loop_inference(number_iter, test_duration)(inference_iteration)(get_slice, | ||
input_info, predictor) | ||
else: | ||
result, exec_time = inference_with_output(get_slice, input_info, predictor, outputs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я не уверена в необходимости функции inference_with_output. См. комментарии ниже.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IgorKonovalovAleks, в целом все хорошо, можно двигаться дальше.
…dlepaddle_inference # Conflicts: # src/inference/inference_paddlepaddle.py
<InputName>inputs</InputName> | ||
<OutputNames></OutputNames> | ||
<ThreadCount></ThreadCount> | ||
<InterOpParallelismThreads></InterOpParallelismThreads> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
За что отвечает параметр InterOpParallelismThreads
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вопрос, можно ли настроить параметры параллелизма (число потоков и другое) при выводе библиотеки?
<ThreadCount></ThreadCount> | ||
<InterOpParallelismThreads></InterOpParallelismThreads> | ||
<IntraOpParallelismThreads></IntraOpParallelismThreads> | ||
<KmpAffinity></KmpAffinity> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
За что отвечает параметр KmpAffinity
?
@IgorKonovalovAleks, надо обновить 4 ридми-файла:
|
@valentina-kustikova версия финальная, можно проверять |
docker/PaddlePaddle/Dockerfile
Outdated
ARG PADDLE_VERSION=2.5.2 | ||
RUN pip3 install --upgrade pip && \ | ||
pip3 install opencv-python requests PyYAML docker | ||
RUN export LD_LIBRARY_PATH=/root/miniconda3/lib:${LD_LIBRARY_PATH} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не очень уверена, что у нас всегда установка будет проходить сюда. @ismukhin, что скажете?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
По смыслу этой строки быть не должно. У нас сейчас явно miniconda
ставится только для Caffe
, чтобы работать в нужном окружении с архивными пакетами.
self.input_name = None | ||
self.output_names = None | ||
self.nthreads = None | ||
self.delegate = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кажется, что этого параметра нет при запуске инференса с помощью PaddlePaddle. Проверьте, пожалуйста.
self.output_names = None | ||
self.nthreads = None | ||
self.delegate = None | ||
self.delegate_options = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Аналогичный комментарий.
src/inference/README.md
Outdated
@@ -15,6 +15,7 @@ | |||
1. Deep Graph Library (DGL, PyTorch-based). | |||
1. RKNN. | |||
1. ncnn. | |||
2. PaddlePaddle. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Почему 2. вместо 1.?
src/inference/README.md
Outdated
между блокирующими операциями. По умолчанию не установлен и выбирается | ||
TensorFlow автоматически. | ||
- `--raw_output` - работа скрипта без логов. По умолчанию не установлен. | ||
- `--restrisct_gpu_usage` - ограничение использования видеокарт до 1-й. По умолчанию не установлен. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Также параметра нет. Проверьте, пожалуйста, корректность всего списка параметров.
type=bool, | ||
dest='raw_output') | ||
parser.add_argument('-nthreads', '--number_threads', | ||
help='Number of threads to use for inference on the CPU. (1 by default)', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Удалите, пожалуйста, точку перед скобкой.
help='Number of threads to use for inference on the CPU (1 by default)'
dest='report_path') | ||
parser.add_argument('--time', required=False, default=0, type=int, | ||
dest='time', | ||
help='Optional. Time in seconds to execute topology.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Удалите, пожалуйста, точку в конце предложения перед закрытием кавычки
@IgorKonovalovAleks, посмотрела код, написала замечания. Там еще висят открытые вопросы с прошлого ревью, отработайте их, пожалуйста. Спасибо! |
docker/PaddlePaddle/Dockerfile
Outdated
ARG PADDLE_VERSION=2.5.2 | ||
RUN pip3 install --upgrade pip && \ | ||
pip3 install opencv-python requests PyYAML docker | ||
RUN export LD_LIBRARY_PATH=/root/miniconda3/lib:${LD_LIBRARY_PATH} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
По смыслу этой строки быть не должно. У нас сейчас явно miniconda
ставится только для Caffe
, чтобы работать в нужном окружении с архивными пакетами.
docker/PaddlePaddle/config_tf2.yml
Outdated
@@ -0,0 +1,30 @@ | |||
models: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нужно переименовать конфиг
docker/PaddlePaddle/Dockerfile
Outdated
|
||
RUN python3 setup.py install_core | ||
|
||
RUN accuracy_check -c config_tf2.yml -m ../../../resnet50 -s sample |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Копипаст имени файла конфига из tf2
, предлагаю заменить имя на config_pdpd.yml
или же просто config.yml
.
src/benchmark/README.md
Outdated
@@ -21,6 +21,7 @@ the following frameworks: | |||
- [Apache TVM][tvm] (Python API). | |||
- [RKNN][rknn]. | |||
- [Spektral][spektral] (Python API). | |||
- [PaddlePaddle][paddlepaddle] (PythonAPI). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python API (пробел нужно вставить)
@valentina-kustikova поправил замечания |
|
||
## Public models (Open Model Zoo) | ||
|
||
Represented deep models are available in [Open Model Zoo][omz]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь какой-то баг, либо ссылка неправильная, либ это не Open odel Zoo.
-|-|-| | ||
resnet-50 |Mean values - [123.675,116.28,103.53],<br>scale value - [58.395,57.12,57.375]|0.9931559 Granny Smith<br>0.0009120 piggy bank, penny bank<br>0.0007721 bell pepper<br>0.0007689 tennis ball<br>0.0005548 candle, taper, wax light| | ||
PPLCNet_x1_0_infer |Mean values - [123.675,116.28,103.53],<br>scale value - [58.395,57.12,57.375]|0.2785943 Granny Smith<br>0.2241544 piggy bank, penny bank<br>0.0404602 saltshaker, salt shaker<br>0.0131707 soap dispenser<br>0.0114298 lemon | ||
| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Уехал разделитель на новую строку почему-то
@IgorKonovalovAleks, не все и не везде :) написала. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IgorKonovalovAleks, осталось пара вопросов.
|
||
|
||
def main(): | ||
args = cli_argument_parser() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IgorKonovalovAleks, здесь вопрос также открыт.
@ismukhin, посмотрите, пожалуйста. Если все хорошо, то мержите. |
|
||
log.info(f'Performance results:\n{json.dumps(inference_result, indent=4)}') | ||
except Exception as e: | ||
log.warning('uncaught error: {0}'.format(str(e))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше, наверное, единообразно отлавливать и выводить исключения:
log.error(traceback.format_exc())
sys.exit(1)
No description provided.