Mark homework 1, exercise 3 of Peigen Zhou
Closed this issue · 3 comments
Dear Professor @cecileane ,
I just finished my exercise 3, here is the last commit 503118e
There are three updates
- hw1/readme.md
- scripts/summaryizeSNaQres.sh
- results/summary.csv
For this commit,
- log and out files names are all changed.
- results/summary.csv has been created
For testing all the scripts work well, you can use following commands from terminal in hw1 directory.
make clean
make
Thanks Peigen! This is my way of asking Irene (@anqis929) and Chelsey (@ChelseyGreen) to try your script and provide feedback on it.
Hi Paigen @BaconZhou @cecileane ,
Great Work !
I especially like how you undo the file names for us so that we do not have to the change the file names back when doing the other peer review.
The way you used "grep" "sed" shows your logic clearly.
The only thing I think you could improve is adding some comments to each command so that people would know what this line of code was used for.
Irene
Hi Paige @BaconZhou @cecileane
I'm going to list out my feedback since that's the easiest way for me to stay organized.
(1) Wow! I'm impressed by your readme file in your HWK1 directory- you've definitely shown me the markdown formatting I should be striving towards! One small request for addition, I don't think I saw it specified from where the script should be run or the hierarchy of folders your script is assuming. Is that what "working directory" maybe means? I'd also like to see a short readme file in each of your other directories to identify the files that are in it - when they were created and by what script, if applicable.
(2) The script works as prescribed. I like that you put the CSV file in a separate directory. I didn't see however, a warning to the user (in your readme or in the actual code) that you'll be creating this file and directory -- it would overwrite any files that have with that name in an existing directory.
(3) Your use of grep, sed, and cut together required a knowledge of each of the function. You can do most of these calls, however just using the sed command, since it is able to replace everything other than what you don't want (using .*). I can show you this use of sed sometime in class if you'd like.
(4) I like that you thought to only return the loglik values before the period for the integer math. I returned the decimal part and then had to strip it off-- your method is more efficient!
(5) You can streamline your code a bit by nesting the if statements within each other. Then if a lik fails the -le 3460 check, it will automatically bypass the le 3450 and 3440 checks. I don't think doing 2 extra if statements will hurt performance much if any especially with the number of values in loglik, but it's a way to make your code a bit more compact.
(6) I'd recommend you add some comments at the top of your code to explain what it does and throughout the code so its easier for a user to understand the steps you have taken.
Please let me know what questions you have on my feedback and suggestions.
Thanks for letting me read through your code!
Chelsey