smiths / caseStudies

Case studies of (manual) documentation for scientific computing software
3 stars 2 forks source link

MIS issues #218

Closed Malavika-Srinivasan closed 4 years ago

Malavika-Srinivasan commented 5 years ago

@bmaclach

Hi Brooks,

Please see some of the small things I noted in MIS.

  1. In some of the places such as section 8.3.2, you use [R] ^ {1,2}. Does it indicate a matrix? because I also see [R] ^{1 X 2}. Can you please clarify?

  2. In section 8.4.4, you are comparing last 5 characters of your filename and checking for only ".out", which is 4 characters, please verify it is correct or maybe it needs to be modified to last 4 characters. Just double check please.

  3. In section 9.2.1, the name of the slip weighter module is mentioned as SlipWeighter but in the output in section 9.4.4, it says weighter(slip surfs)[0].surf..., should it say slipWeighter.weighter?

  4. In section 4.3, I think your second assumption can be verified rather than assumed. Please think about it.

  5. [R] this is out of curiosity, do the square bracket mean anything?

  6. In section 15.3.3, this is not really a issue, just small things - The column width for `In', if changed will look better :)

  7. In page 27, I guess you forgot a `\'. It says coloneqq in 17.4.4.

It is a very well done document @bmaclach.

Thank you,

Malavika

bmaclach commented 5 years ago

Thanks @Malavika-Srinivasan , those are a lot of good catches! To address each of your points:

  1. Yes, the "1,2" is intended to be the dimensions of the matrix or array. Initially I was using "1x2" before realizing that the notation in Hoffman and Strooper was different. Looks like I forgot to fix some of the ones that were using the wrong notation. Fixed now in b017a1e1d3fa191b8c055e55016e5e4432a220e8

  2. Great catch! Fixed in b017a1e1d3fa191b8c055e55016e5e4432a220e8

  3. I was thinking of SlipWeighter simply as a short name for referring to the module, whereas weighter is the name of the method. I did not think I was supposed to preface the method call with the module name. If I am supposed to do that, I will need to change it throughout the document, as I did the same thing with other modules. Maybe @smiths can provide an answer here.

  4. You are right. The way I did it is a reflection of the current implementation, where that is assumed. Given I will be updating the implementation, it makes sense to add a new check and exception for this. So I've updated the MIS to reflect that in b017a1e1d3fa191b8c055e55016e5e4432a220e8

  5. The square brackets mean that it is a sequence. (For example, [R] is a sequence of real numbers)

  6. I agree. Fixed in b017a1e1d3fa191b8c055e55016e5e4432a220e8

  7. Fixed in b017a1e1d3fa191b8c055e55016e5e4432a220e8

smiths commented 5 years ago

@bmaclach, you are correct that you do not need to preface methods calls with module names, unless there is ambiguity. I haven't checked, but I assume that the method is either being used in its own module, or there is a uses clause to "import" it.