Skip to content
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

Regression Test repo pull request #232

Merged
merged 153 commits into from
Aug 2, 2023
Merged

Conversation

asyatrhl
Copy link
Collaborator

@asyatrhl asyatrhl commented Jun 8, 2023

Regression test codes are created for develop branch of MaximIntegratedAI/ai8x-training repository.

@@ -88,7 +88,7 @@ PascalVOC_2007_2012_256_320_augmented:
threshold: -500
epoch: 15
Kinetics400:
datapath: "/data_ssd"
datapath: "/data/alican_data/"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a generic destination and update documentation

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fixed.

- name: Create Test Script
run: python ./regression/create_test_script.py --testconf ./regression/test_config.yaml --testpaths ./regression/paths.yaml
- name: Run Training Scripts
run: bash /home/test/actions-runner/_work/ai8x-training/ai8x-training/scripts/output_file.sh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where does _work originate?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is default working directory of the github runner. Name of this directory can be changed while installing for the MaximIntegratedAI:develop repo.

output_file_path = pathconfig["output_file_path_onnx"]
train_path = pathconfig["train_path"]

logs_list = folder_path + '/' + sorted(os.listdir(folder_path))[-1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be os.path.join()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fixed.

temp_path = logs_list + "/" + file_p
for temp_file in sorted(os.listdir(temp_path)):
if temp_file.endswith("_checkpoint.pth.tar"):
temp = f"{temp_path}/{temp_file}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.path.join()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fixed.

path_command = "cd " + local_path
subprocess.run(path_command, shell=True, check=True)

source_path = "/home/asyaturhal/actions-runner/_work/ai8x-training/ai8x-training/logs/"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not use a hard coded personal home here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fixed.

files_old_temp = files_old.split("___")[0]
if files_old_temp == files_new_temp:

old_path = old_logs_path + '/' + files_old
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.path.join() ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fixed.

pathconfig = yaml.safe_load(file2)

log_path = pathconfig["log_path"]
log_path = log_path + '/' + sorted(os.listdir(log_path))[-1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.path.join()

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fixed.

threshold_temp = float(config[f'{log_data}'][f'{log_model}']['threshold'])
else:
threshold_temp = 0
logs = log_path + '/' + str(logs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...

threshold: -500
epoch: 15
Kinetics400:
datapath: "/data/alican_data/"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid personal directories

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fixed.

@asyatrhl asyatrhl requested a review from rotx-eva July 10, 2023 18:40
Copy link
Contributor

@ermanok ermanok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems it is OK after a few minor updates I mentioned.


## Creating Test Scripts

The sample training scripts are under the `scripts` path. In order to create training scripts for regression tests, these scripts are rewritten by changing their epoch numbers by running `regression/create_test_script.py`. The aim of changing the epoch number is to keep the duration of the test under control. This epoch number is defined in `regression/test_config.yaml` for each model/dataset combination. Since the sizes of the models are different, different epoch numbers can be defined for each of them in order to create a healthy test. If a new training script is added, the epoch number and threshold values must be defined in the `regression/test_config.yaml` file for the relevant model.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'Since the sizes of the models are different' can be changed as 'Since the sizes of the models and the datasets are varying'

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

test_path = args.testpaths

# Open the YAML file
with open(yaml_path, 'r', encoding='utf-8') as file:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better naming can be used for 'file' & 'file2'

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'file' and 'file2' updated as 'yaml_file' and 'path_file'.

@@ -0,0 +1,130 @@
###################################################################################################
#
# Copyright (C) 2023 Maxim Integrated Products, Inc. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The copyright should be for Analog Devices Inc. not for Maxim Integrated Products.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copyright updated.

Copy link
Contributor

@ermanok ermanok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@rotx-eva rotx-eva merged commit 6e21e47 into analogdevicesinc:develop Aug 2, 2023
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants