Conversation
…dded-dev-research/itlab_2023 into Semyon1104/Acc_Parametrized
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #277 +/- ##
=======================================
Coverage 84.49% 84.49%
=======================================
Files 59 59
Lines 3632 3632
Branches 2184 2184
=======================================
Hits 3069 3069
Misses 278 278
Partials 285 285 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
aobolensk
left a comment
aobolensk
left a comment
There was a problem hiding this comment.
General: please make pre-commit check smaller and leave full scope testing for nightly
app/Graph/acc_check.cpp
Outdated
| if (num_photo < 1 || num_photo > 50000) { | ||
| std::cerr << "Warning: num_photo should be between 1 and 10000 " |
There was a problem hiding this comment.
there are 50k photos in the dataset, and in theory, you can specify 50k
There was a problem hiding this comment.
Please, fix the Warning string
app/Graph/acc_check.cpp
Outdated
| std::cerr << "No images found in dataset path: " << dataset_path << '\n'; | ||
| return 1; | ||
| } | ||
| size_t images_per_class_base = num_photo / 1000; |
There was a problem hiding this comment.
num of output classes
There was a problem hiding this comment.
Can we create a variable (constant) with that name in the code and use it?
| accuracy_value_${{ matrix.model }}.txt | ||
|
|
||
| - name: Update README for model (master only) | ||
| if: github.ref == 'refs/heads/master' |
There was a problem hiding this comment.
Our main branch is called main this code will not be run whatsoever
app/Converters/parser_onnx.py
Outdated
|
|
||
| parser = argparse.ArgumentParser(description='Конвертация моделей в JSON формат') | ||
| parser.add_argument('model_name', type=str, | ||
| choices=['googlenet', 'densenet', 'resnet', 'yolo', 'alexnet'], |
There was a problem hiding this comment.
"alexnet" is not present in model_files and output_files dict keys. Is that expected?
| else | ||
| echo "Плейсхолдер для ${{ matrix.model }} не найден в README, добавляем в конец" | ||
| echo -e "\n## ${{ matrix.model }} Accuracy\n<!--ACCURACY_${{ matrix.model }}_PLACEHOLDER-->Accuracy: ${ACCURACY}% (updated: ${DATE})<!--END_ACCURACY_${{ matrix.model }}-->\n" >> README.md | ||
| fi |
There was a problem hiding this comment.
It is better to report an error if we didn't find something
| ACCURACY=$(cat accuracy_value_${{ matrix.model }}.txt | sed 's/%//g') | ||
| DATE=$(date '+%Y-%m-%d') | ||
|
|
||
| if grep -q "<!--ACCURACY_${{ matrix.model }}_PLACEHOLDER-->" README.md; then |
There was a problem hiding this comment.
Should we update README in that regard? Since we have more model and only one placeholder as of now
app/Graph/acc_check.cpp
Outdated
| options.threads = std::stoi(argv[++i]); | ||
| } else { | ||
| try { | ||
| num_photo = std::stoi(argv[i]); |
There was a problem hiding this comment.
Shouldn't we inctrment i here as it is already done in the other options?
There was a problem hiding this comment.
I don't understand, we'll have a loop iteration, and i will be correctly assigned to numPhoto. We don't specify --numPhoto
There was a problem hiding this comment.
I'm just not sure why it differs from other optons handling
There was a problem hiding this comment.
The difference is that num_photo is a positional argument, not a flag with a parameter. For other options (--model, --threads, etc.), we first read the flag and then its value. For num_photo, we simply read the number directly.
…dded-dev-research/itlab_2023 into Semyon1104/Acc_Parametrized
…dded-dev-research/itlab_2023 into Semyon1104/Acc_Parametrized
3 participants
No description provided.