1012 bug in getting testparameters in specific test derived from generictest

[khoanguyen-dev]

Changes and Information

Please briefly list the changes (main added features, changed items, or corrected bugs) made:

• Change TestType to enum class
• Create TestData struct to get TestParameter for each type
• Add a test to test the get_default()
• Delete the GenericTest and other type
• Modify the previous tests to accommodate these changes

If need be, add additional information and what the reviewer should look out for in particular:

• The changes are mostly made in parameters.h
• The TestData is added as one of the World parameters and TestParameters are extracted when they are needed.

Merge Request - Guideline Checklist

Please check our git workflow. Use the draft feature if the Pull Request is not yet ready to review.

Checks by code author

• [x] Every addressed issue is linked (use the "Closes #ISSUE" keyword below)
• [x] New code adheres to coding guidelines
• [x] No large data files have been added (files should in sum not exceed 100 KB, avoid PDFs, Word docs, etc.)
• [x] Tests are added for new functionality and a local test run was successful (with and without OpenMP)
• [x] Appropriate documentation for new functionality has been added (Doxygen in the code and Markdown files if necessary)
• [ ] Proper attention to licenses, especially no new third-party software with conflicting license has been added
• [ ] (For ABM development) Checked benchmark results and ran and posted a local test above from before and after development to ensure performance is monitored.

Checks by code reviewer(s)

• [x] Corresponding issue(s) is/are linked and addressed
• [x] Code is clean of development artifacts (no deactivated or commented code lines, no debugging printouts, etc.)
• [x] Appropriate unit tests have been added, CI passes, code coverage and performance is acceptable (did not decrease)
• [x] No large data files added in the whole history of commits(files should in sum not exceed 100 KB, avoid PDFs, Word docs, etc.)
• [ ] On merge, add 2-5 lines with the changes (main added features, changed items, or corrected bugs) to the merge-commit-message. This can be taken from the briefly-list-the-changes above (best case) or the separate commit messages (worst case).

Closes #1012

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.22%. Comparing base (575a146) to head (c837eb9). Report is 7 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1013 +/- ## ========================================== - Coverage 96.23% 96.22% -0.01% ========================================== Files 128 128 Lines 10852 10869 +17 ========================================== + Hits 10443 10459 +16 - Misses 409 410 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[khoanguyen-dev]

There is still an error in the Pycode. The mio::CustomIndexArray<mio::abm::TestParameters, mio::abm::TestType> could not be bound since the pymio::bind_CustomIndexArray function does not include an overload for a tuple of doubles (just a single double). I will try to work to extend this. If time is essential, I can set up the TestParameters in the test directly instead of taking it from the world.parameters.

[reneSchm]

According to CodeCov the coverage dropped because of some "indirect changes" by commit a8dd034 (I am not sure that the branch is reported correctly), so now line 200-202 in cpp/models/abm/infection.cpp are no longer covered. Probably some parameter change in the tests could fix this, but maybe have a look at the function as well (it might possibly need updating?).

[DavidKerkmann]

It seems that I managed to fix the python bindings. However, I had to implement serialise and deserialise for the simple

struct TestParameters {
     UncertainValue<> sensitivity;
     UncertainValue<> specificity;
}

as it couldn't be detected by the templates in io/io.h. Is there a way around that? I can imagine letting this derive from std::pair and write access operators for .first and .second but there may be something more simple. Please comment if you have ideas.

[reneSchm]

Is there a way around that?

Not really, we cannot iterate over members of generic structs. (Technically, we could using a void*, but that will cause more issues than it solves.)

I can imagine letting this derive from std::pair and write access operators for .first and .second

That (or tuples, vectors, or other types with its own (de)serialize_internal function) would work, but you lose the names when writing the Json.

I'd say the best options are

• Write the (de)serialize functions. It is not too much work, and does everything you need.
  • I am also working on a shorter serialization feature in the branch connected to branch 652, see here for example.
• Replace the TestParameter completely by a std::pair. You can use std::tie or structured binding to set the names, e.g.

  auto& [sensitivity, specificity] = params.get<GenericTest>();

[DavidKerkmann]

I see. In that case I would leave it as is for now and make changes later if we need to.