yungyuc / nsdhw_20sp Goto Github PK

In hw3 instructions, we are asked to output performance.txt,
while validate.py would output another performance.txt (see here).
I guess validate.py would overwrite performance.txt later.

I would like to propose a enhancement on action CI. It would be nice to slightly change hw1.yml to this. The main difference is to replace $GITHUB_REF to $GITHUB_REPOSITORY.

With the change, we'll be able to test ourselves on our repository. Previous way getting $username using $GITHUB_REF are not able to achieve this. In the previous test, the action is passes since the branch name is 'yungyuc-hw1-submit-example', but in most cases the branch may not be this format (hw1, hw2, etc.). Using $GITHUB_REPOSITORY will still do the same trick but allowed students to test individually on their own forked repository. To avoid breaks after the change, I listed the possible conditions came up my mind in the below.

1. [Behaviour changed!] If the push is from the collaborator or the owner of base repo, e.g. @tigercosmos push a commit to yungyuc/nsdhw_20sp on branch hw1. The change should work well considering the output of $GITHUB_REPOSITORY in this test.

2. [Behaviour changed!] If the push is from owner of forked repo to the forked repo, e.g. @yanagiragi push a commit to yanagiragi/nsdhw_20sp on branch hw1. The changes worked well in here.

3. [Behaviour changed!] If the PR is from owner in base repo, e.g. @yungyuc send a PR from yungyuc/nsdhw_20sp branch whatever to yungyuc/nsdhw_20sp on branch hw1. The change should work well considering the output of $GITHUB_REPOSITORY in this test.

4. If the PR is from owner in forked repo, e.g. @yanagiragi send a PR from yanagiragi/nsdhw_20sp to yungyuc/nsdhw_20sp on branch hw1. The change did not break in here.

Condition 1 and 3 has not been tested since I do not have the permission. Current solution does not support the collaborator to submit tests on it directiory (e.g. @tigercosmos wish to test hw1.yml action with files in hw1/tigercosmos on yungyuc/osd_20sp branch hw1 ). The collaborator might need to fork the project and test the action CI like other students.

@tigercosmos Do you want to merge hw1 branch back to master?

It should be validate.py, but the CI script wrongly put it as validate.sh.

When the validating script uses bash, if the users set the locale other than C or en, the error message may not be in English and the script fails detection.

see the log in https://github.com/yungyuc/nsdhw_20sp/runs/500966947
seems when the error occurs in the sub-process, the CI cannot find it.
cc @yungyuc

Some question about validate.sh in hw2 repo.

• What do the scripts at line 115~117 want to test?
  • May I ask which package contains the _vector module? or _vector should be generated by pybind?

echo "import the extension module:"
python3 -c 'import _vector ; print(_vector)' ; ret=$?
if [ $ret -ne 0 ] ; then echo "failure" ; exit 1 ; fi

• Why are there exit 0 at line 140 and at line 145?
  • It seems that these two exit 0 might skip the following validating scripts.

https://github.com/yungyuc/nsdhw_20sp/blob/hw3/hw3/validate.py#L179

This validation program will measure the performance and output "performance.txt".

Do we still need to do measurement ourselves as mentioned in the 4th grading spec ?
Also, if we output "performance.txt", this file will be over-written by that validation program.

4. Time the two multiplication functions and compare the performance. Then output performance.txt. The format of performance.txt can be any. Just make it reasonable. (1 point)

Thanks~

@tigercosmos could you test if CI is working? PR #4 ran into a bug that I tried to fix with #5, but to fully validate my fix another GitHub account is needed.

You may follow what I did in #6 for the test.

It seems to be missing a '?" in here ? I guess it should be '$?' instead.

Log:

./validate.py: line 35: [: $: integer expression expected