-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Peer review from @NavonilNM
This continues from #44, #62, #130, #149 and #150.
Verification and validation
-
Thanks for adding a JOS reference to the V&V section (Sargent, 2013).
-
Information boxes “What does it mean in practice?” – great idea, with links to content covered in the eBook.
-
Boundary value testing: KLE’S Nijalingappa College (2025). Is a slightly odd reference. We do not know the authors. Nor can we identify the module. We probably know that it comes from Bachelor of Computer Application (BCA) - https://bca.klesnc.edu.in/wp-content/uploads. Perhaps we can find some other citable source; or of not, could we just provide a footnote with the link and last access date?
-
Minor:
• Patients below the minimum are rejected (17 or under).
• Patients at/near the thresholds are accepted (e.g., 18, 19, 74, 75).
• Patients above the maximum are rejected (76 or over). -
Utilisation: 0-1 (OK). Will it help add percentages as well (0-100%)?
-
Regression Testing: (Balci, 1998) – Is this the source?
Learners from Business and Management disciplines, they may be curious to know how does the technique in M&S align with regression they may have done in their RM courses (i.e., examining the relationship between dependent and independent variables). I looked into the chapter to learn more, but there are only seven instances of the term:
You can include it if you like; but no harm omitting or looking at other sources?
-
Minor: Face validation. I would reverse the order of bullet points – start with people familiar with real system, then intended users and then project members (the latter may developed the model in any case).
-
Graphical comparison: Slight rewording: “..involves comparing graphs of model variables over time with graphs of real system variables.” --> ..involves comparing graphs of model results over time with graphs of actual observed outcomes “. (for consistency) – check “predictive validation
-
Graphical comparison: For the highlight box, slight rephrasing of the sentence may be needed.
-
Animation Visualisation: “Creating an animation takes time..” perhaps add using FOSS? But it may be self-explanatory, considering the nature and the intended purpose of the eBook. In relation to the sentence, “Communication: It is really helpful for sharing the models with others (such as with stakeholders and decision maker..” (Great!), but probably we can add that it supports discussion and communication between stakeholders .. something like that.
-
Comparison testing: Excellent. What better example than your book 😊
-
Input data validation – I was thinking whether it possible to slightly reorder the validation approaches as per the lifecycle of a M&S study (so, first we have conceptual model validation, the input data validation, … ending with face validation, cross validation, etc.?). Ok, it is not always such a clear-cut step-by-step approach, sometimes (more often than not) steps are skipped; sometimes there is a feedback and repeat..but good to have a logical flow which tags on to the modelling life cycle.
-
Cross validation: Highlight box – not sure what we meant by “methods”..
-
General Comment: We are referring to the two example models in several section. It’s great for learning. However, for the Verify elemet will the readers expect to land on a webpage that explains, with reference to the two example models, the verification approaches implemented – like Jupyter notebook style (like what you have done for some other sections – what to look out for? and referring to particular files). This is probably out of scope of this work, but just wanted to mention it.
-
Checklist – excellent. Would be good to know how to use it? (e.g., is it a good practice to include it in the repo?). What does a MD document stand for and it’s significance (verification_validation_checklist.md)?
Test
- Learning objectives: Rephrasing can help “..likewise, also used on the scenario and sensitivity analysis page)” – what is also used?
Quick check: Pre-reading sequence (below)?
-
Suggestion: Identify issues (rather than catch issues)?
-
I have not used pytest, so was a bit confused with the function below. I am sure it is right, also there is the asset statement (which would definitely have a special meaning); however number=5 is something I would have expected as an argument to be passed? (I may understand what is happened as I scroll through the page). But for now, at this point, I do not understand it; perhaps a comment can help?
- Screenshots from the two examples in this section; can the comments be slightly edited (for consistency) or perhaps I am reading the assert statement wrong?
- View Simulation Code: I guess the code from parallel processing is being repeated here (OK); just confirming because in previous sections you had “grey” for existing code and “black” for new code
- For the first test (example), it seems to work but ExitCode.OK:0 was not printed (as shown in your example).
However, when I changed the number to a negative value it reported error (see below).
-
The second example (parameterised example worked fine). P.S: In reference to comment (3) of this section (starts "I have not used pytest") – I now understand it better; however, this is only after looking at the second example and the @pytest.mark.parametrize("number", [1, 2, 3, -1]) directive.
-
Functional tests: 1st test worked with the following changes:
• Imported directly from the files the classes Parameters and Runner (circled in red below)
• From package root Pytest simulation/tests/test_functional.py (appended package name “simulation” to tests)
- Functional tests: 2nd test (test_zero_inputs) passed? Expected it to fail? It the example screenshot also it has passed?
- Back tests: It worked. However, I think “import pytest” may need to be added to the example.
- I found the following text (see below) a bit difficult to follow; perhaps an illustration could help?
Perhaps provide an example to show when it fails; for example, I changed IAT to 4 (from 5) and it failed when comparing with the pre-generated set of expected results.
-
The sentence: “See the GitHub actions page for guidance on generating a coverage badge for your README within CI/CD” perhaps also indicate that it will be covered in the next section on style and documentation. Otherwise, the page opens, but difficult to understand the test coverage example. I guess, working through the next section, it will become clear.
-
Suggestion: Rather than stating “A wide variety of tests are available ..”, perhaps we can perhaps be specific on the tests available under the two example model (backtest, functionaltest and unitest). This is similar to the sentence you have for back test data, where specific notebook has been included (generate_exp_results.ipynb).
-
Finally, the following sentence may need to be explained further:
“Tests are run and coverage are calculated on pushes to main via GitHub actions (.github/workflows/tests.yaml).
Mathematical proof of correctness
-
Learning objectives: Rephrasing can help “..likewise, also used on the scenario and sensitivity analysis page)” – what is also used?
-
The “small example we’ve been building..”; does “simple example” read better?
-
“The model used is the same as that on the parallel processing page - as also used on the scenario and sensitivity analysis and tests pages.” I guess, “as also” refers to the model?
-
Good explanation (below).
- Do we need R code? I got lost here.
Would it better to define a separate Python function? I am not running this part of the code and the tests for comparing simulation with mathematical model results. I think these will not run.
Quality Assurance
-
scoping.md – Does it have to be a .md file. What is the benefit of .md file?
-
This section reminds me of a paper Alison and I wrote on Facets of Trust for Simulation Studies.
https://www.sciencedirect.com/science/article/pii/S0377221720306019
We wanted to carry forward this work from the perspective of QA; that is, the translation from the conceptual to the QA-based structured approach with different role like analyst and approver (in the US, companies like MITRE act as assurers for large govt. contracts; also MTC have a separate assurance team); anyway, we got drawn into other things. The strategy unit project is a good example!
Overall very good section and I learned about the automated test method! Thanks for the great work!
Metadata
Metadata
Assignees
Labels
Type
Projects
Status