-
Notifications
You must be signed in to change notification settings - Fork 727
Update README.md #1
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
Conversation
@@ -6,5 +6,74 @@ | |||
"languages": [{"cpp": {"properties": {"projectOptions": [{"projectType": "makefile"}]}}}], | |||
"targetDevice": ["CPU", "GPU", "FPGA"], | |||
"os": ["linux", "windows"], | |||
"builder": ["ide", "make"] | |||
"builder": ["ide", "make"], | |||
"configurations": { |
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.
Roman -- I think the name of the CI testing object should be more direct. The word configurations
does not communicate to me that this contains CI tests. How about calling the object ciTests
?
"configurations": { | |
"ciTests": { |
{ | ||
"id": "cpu_usm", | ||
"build": [ | ||
"make all" | ||
], | ||
"run": [ | ||
"make run" | ||
], | ||
"clean": [ | ||
"make clean" | ||
] | ||
}, |
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.
Roman -- I think my suggestion (below) is consistent with what you were alluding to, yesterday. Rather than being specific to the build, you might want to have a more general set of "phases" that direct the tests to be performed (mind you, I'm not a CI expert, so take my suggestions with a "grain of salt.").
Perhaps something like the following, which are overly complex to make a point, not to indicate that all tests would be like this (and where all array element "actions" are assumed to start from the root of the sample):
"linux": [
{
"id": "cpu_usm",
"setup": [
[ "mkdir -p test/test && cd test/test && echo 'x=y' >test.txt", "/success-regex/", "/fail-regex/" ],
[ "export TEST=test ; touch abc ; my-setup-script.sh || true", "/success-regex/", "/fail-regex/" ]
],
"test": [
[ "mkdir build && cd build && cmake ..", "/success-regex/", "/fail-regex/" ]
],
"clean": [
[ "make clean", "", "" ]
]
}
],
Where the idea is that a sequence of commands that could be executed at the shell, with optional "success" and "fail" regex strings provided to spot a success or failure based on the output (if any) of the commands. I've made the above overly complex just to make the point that I don't think you want to assume a particular build tool, rather that you just want to know "what do I type to setup for a test" and "what do I type to test" and "what key phrases/output" do I expect to see on success/failure?
I would also assume that you would continue to check the return code from each "action" as part of the testing, assuming that a non-zero value is a pass and anything else is a fail...
Anyway, just a thought, any further than what I have suggested above you'd probably want to use something like YAML rather than JSON to use as the testing template.
Again, I emphasize that this is just an idea. :)
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.
What goes into the test command line is completely up to the sample owner. If they want to validate output against a regex, they can do it as long as the command returns zero on success and non-zero otherwise. I don't think we can reliably tell in CI whether the sample is running well other than by checking the exit status.
I'm still not sure how to break down the test commands, by build/run/clean stages or by test phases as you suggested or have them as a single command at all. Or there can be arbitrary stages. For example some samples may omit the clean
step, others may include more steps, e.g. pre-build
but I'm worried it may complicate the things.
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.
Header
Test
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.
Change request
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.
Is this good to merge?
…torials. (#553) * First commit for merge sort design Signed-off-by: tyoungsc <[email protected]> * Cleaned up MergeSort.hpp a bit using defines Signed-off-by: tyoungsc <[email protected]> * Formatting/comments * Reduced area by connecting Partition unit (previously called Shuffle) to only the first merge unit Changed Shuffle to Partition Updated pictures and README Used existing pipe_array code (instead of using my own) Code cleanup and comments Tested in emulation and reports, doing a HW build now * Fixed bug with no-USM BSPs in Produce kernel Slight code cleanup * README update after Mike's review CMake update Removed line from samples.json that was not necessary Deleted unused files * Changed Windows specific flag to use '/' instead of '-' * CMake update * Removed all pointer arithmetic for offseting to the inside of kernels (to avoid runtime issue) * Big change to example design: encorperate a new multi-element per cycle merge unit, use a bitonic sorter on the input, rather than a simple partition. Updated README and pictures to fit the new design. Addressed Mike's most recent review comments * Code cleanup: changed some variable names and comments * README update * Picture update * Comments * Code cleanup and comments * Small change to README * Merged shannonization VCXPROJ files into a single one (as per all the other tests). Small update to source file to change II target for A10 and fix indenting * Adding file I missed in previous commit * Updated comment * Grammar update * Again * Updating design output and README * Simplified Produce kernel * Allowed SORT_WIDTH to be 1 (1 element per cycle) Improved Merge kernel for case where SORT_WIDTH=1 with shannonization * Code cleanup Variable renaming Comments Grammar * Comments, formatting, README grammar changes, and general cleanup :) * Renamed all files to match google style Renamed main file to main.cpp Used impu namespace in unrolledloop, pipearray, and static_math Renamed static_math.hpp to impu_math.hpp Updated README, Windows VS files, and CMake with these changes * README updates Comments * Changed filenames to use underscores between words * Added 'pipe' namespace to impu namespace for pipe utilities * Revert "Merge pull request #1 from tyoungsc/new_fpga_ref_design_merge_sort" This reverts commit 202ff82, reversing changes made to 72580ac.
Initial draft for JAX README.md
No description provided.