Skip to content

Add vtr_reg_weekly_no_he that drops the high effort Titan tests and gaussian blur #1118

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

Closed
wants to merge 2 commits into from

Conversation

litghost
Copy link
Collaborator

@litghost litghost commented Feb 6, 2020

Description

This PR is attempting to create a weekly that can provide the vast majority of the signal, without the excessive runtime.

Related Issue

Motivation and Context

This PR is attempting to do two things:

  • Return QoR / wallclock / memory usage in ~12 - 24 for purposes of quality comparisons

How Has This Been Tested?

Types of changes

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

@probot-autolabeler probot-autolabeler bot added build Build system infra Project Infrastructure lang-cpp C/C++ code lang-make CMake/Make code libarchfpga Library for handling FPGA Architecture descriptions libvtrutil tests VPR VPR FPGA Placement & Routing Tool labels Feb 6, 2020
@probot-autolabeler probot-autolabeler bot added the VTR Flow VTR Design Flow (scripts/benchmarks/architectures) label Mar 3, 2020
@probot-autolabeler probot-autolabeler bot added lang-shell Shell scripts (bash etc.) scripts Utility & Infrastructure scripts labels Mar 3, 2020
@litghost
Copy link
Collaborator Author

litghost commented Mar 5, 2020

@kmurray Dropping the high effort titan tests (and gaussian blur) and running with 3 cores means that the "weekly" test set returns in ~40 hrs, which isn't bad!

I've added a CPU and memory load log every 5 minutes in this PR, which will hopefully identify why the CI takes "forever" if a large memory regression is introduced.

@litghost litghost requested a review from kmurray March 5, 2020 14:52
@litghost
Copy link
Collaborator Author

litghost commented Mar 5, 2020

@kmurray : Is there a better way to compose the vtr_reg_weekly and vtr_reg_weekly_no_he with less duplication?

@litghost litghost requested a review from vaughnbetz March 5, 2020 14:54
@litghost litghost changed the title WIP: Adjust vtr_reg_weekly with less effort on Titan, and remove gaussian blur Add vtr_reg_weekly_no_he that drops the high effort Titan tests Mar 5, 2020
@litghost litghost changed the title Add vtr_reg_weekly_no_he that drops the high effort Titan tests Add vtr_reg_weekly_no_he that drops the high effort Titan tests and gaussian blur Mar 5, 2020
@litghost litghost mentioned this pull request Mar 5, 2020
7 tasks
@kmurray
Copy link
Contributor

kmurray commented Mar 5, 2020

Is there a better way to compose the vtr_reg_weekly and vtr_reg_weekly_no_he with less duplication?

I've considered that, and it's potentially possible but would need some scripting work.

How I'd approach it would be to extend the task_list.txt format to support #include like behaviour, similar to the parse/pass results files. We could the define vtr_reg_weekly_he task_list.txt something like:

%include vtr_reg_weekly/task_lists.basic.txt
%include vtr_reg_weekly/task_lists.he.txt

This would support selectively not running the HE titan test by using the appropriate task list. Selectively dropping a particular benchmark (e.g. gaussianblur) would be more difficult.

@litghost
Copy link
Collaborator Author

litghost commented Mar 5, 2020

This would support selectively not running the HE titan test by using the appropriate task list. Selectively dropping a particular benchmark (e.g. gaussianblur) would be more difficult.

Once we have the include feature, it is straightforward to make 3 task lists.

Do you need the deduplication effort solved right now, or is this PR acceptable?

@vaughnbetz
Copy link
Contributor

vaughnbetz commented Mar 5, 2020

Discussed in meeting. Don't need this PR, and instead just stop doing weekly as a pre-submit (do as post-merge only).
Action to Keith

@litghost litghost closed this Mar 5, 2020
@litghost litghost deleted the faster_weekly branch March 5, 2020 19:33
@litghost litghost restored the faster_weekly branch March 5, 2020 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build system infra Project Infrastructure lang-cpp C/C++ code lang-make CMake/Make code lang-shell Shell scripts (bash etc.) libarchfpga Library for handling FPGA Architecture descriptions libvtrutil scripts Utility & Infrastructure scripts tests VPR VPR FPGA Placement & Routing Tool VTR Flow VTR Design Flow (scripts/benchmarks/architectures)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants