932 Get_default of Parameter HighViralLoadProtectionFactor doesn't work as expected

[khoanguyen-dev]

Changes and Information

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

• Change data type of HighViralLoadProtectionFactor to CustomIndexArray<InputFunctionForProtectionLevel, ExposureType, AgeGroup, VirusVariant>.
• Update python bindings to fix parameter (de)serialize.

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

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

• Every addressed issue is linked (use the "Closes #ISSUE" keyword below)
• New code adheres to coding guidelines
• No large data files have been added (files should in sum not exceed 100 KB, avoid PDFs, Word docs, etc.)
• Tests are added for new functionality and a local test run was successful (with and without OpenMP)
• 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.

---

Benchmark Time CPU Iterations

abm_benchmark/abm_benchmark_50k 2721 ms 2716 ms 1
abm_benchmark/abm_benchmark_100k 5682 ms 5674 ms 1
abm_benchmark/abm_benchmark_200k 11855 ms 11773 ms 1

---

Benchmark Time CPU Iterations

abm_benchmark/abm_benchmark_50k 3087 ms 2953 ms 1
abm_benchmark/abm_benchmark_100k 5698 ms 5690 ms 1
abm_benchmark/abm_benchmark_200k 11585 ms 11554 ms 1

Checks by code reviewer(s)

• Corresponding issue(s) is/are linked and addressed
• Code is clean of development artifacts (no deactivated or commented code lines, no debugging printouts, etc.)
• Appropriate unit tests have been added, CI passes, code coverage and performance is acceptable (did not decrease)
• 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 #932

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.88%. Comparing base (ff9df1e) to head (02e6884).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1068   +/-   ##
=======================================
  Coverage   96.88%   96.88%           
=======================================
  Files         140      140           
  Lines       11502    11504    +2     
=======================================
+ Hits        11144    11146    +2     
  Misses        358      358           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[DavidKerkmann]

Thanks for fixing this. It might also be relevant for SeverityProtectionFactor and InfectionProtectionFactor. Have you checked this?

[khoanguyen-dev]

Thanks for fixing this. It might also be relevant for SeverityProtectionFactor and InfectionProtectionFactor. Have you checked this?

I have checked them (see TEST(TestInfection, getPersonalProtectiveFactor)). I think they are initialised in CustomIndexArray so there is no need for a specified contructor.

[DavidKerkmann]

Ah, yes, they have a different type. Why did we decide again that HighViralLoadProtectionFactor does not depend on age/virus type? Was this a result of some article?

[khoanguyen-dev]

Ah, yes, they have a different type. Why did we decide again that HighViralLoadProtectionFactor does not depend on age/virus type? Was this a result of some article?

Yes, I believe it is based on an article that mentions only about days since onset of symptoms (https://doi.org/10.1093/cid/ciaa886). I guess we could also adopt it to CustomIndexArray with a general age/virus type for consistency.

[DavidKerkmann]

I understand our parameter HighViralLoadProtectionFactor as a protection to obtaining a high viral load when you have been vaccinated or previously infected. From what I can see from the article is only a plot of viral load over time and integral values of this curve which are set into relation with transmission potential, but there is no mention about any previous engagement of the immune system. Am I missing something or could you explain?

From a technical point of view, I agree with you that we should make this dependant on virus variant and age, too, as this might very well differ.

[xsaschako]

I unfortunatley think this is a little convoluted. I think one can just change it to AgeGroup /Agegroup/ as an input parameter and this should work. I think we did this for the paper branch.
Also i think for now just agegroup is fine, let's not make things more complicated as needed right now, and 2 viruses in a simulation (where this would come in handy) is not planned and i think we need to change a greater amount then either way.

[DavidKerkmann]

I unfortunatley think this is a little convoluted. I think one can just change it to AgeGroup /Agegroup/ as an input parameter and this should work. I think we did this for the paper branch.

In the paper branch we just initialised it explicitly.

Also i think for now just agegroup is fine, let's not make things more complicated as needed right now, and 2 viruses in a simulation (where this would come in handy) is not planned and i think we need to change a greater amount then either way.

This is planned in the near future to study the overlap of two (or more) variants and to examine what it takes for one variant to dominate. I also don't think we have to change much in the rest of the codebase, as everything should already work with having two variants. Also, I don't think it created much of an overhead or making things more complicated. I leave the decision to you, @khoanguyen-dev .

[xsaschako]

This is planned in the near future to study the overlap of two (or more) variants and to examine what it takes for one variant to dominate.

Didn't know! Then it's good.

missunderstood

[khoanguyen-dev]

Since I could not find a detailed paper/article mentioning directly how the HighViralLoadProtectionFactor changes over time, some observations from the following paper: https://doi.org/10.1186/s12916-021-02220-0 and https://doi.org/10.1371/journal.pone.0243597:

Data Overview:

• Initial Peak and Decline: Viral load peaks around the onset of symptoms and then gradually declines. The peak typically occurs within the first 5 days of symptom onset, followed by a decline to low levels after about 10 days.
• Protection Factor: The protection factor against high viral load, such as from vaccination or previous infection, can reduce the viral load effectively. This factor generally decreases over time.
  Graph Data Points
  Day 0-5: Viral load starts high at symptom onset, peaks, and begins to decline.
  Day 6-10: Significant decline in viral load as the immune response kicks in.
  Day 11-20: Further decline, with most patients having low or undetectable viral loads by day 20.

I then assumed the data points:

• Day 0: Viral load protection = 0.0 (no protection at onset)
• Day 5: Viral load protection = 0.3 (initial immune response)
• Day 10: Viral load protection = 0.6 (increased immune response)
• Day 15: Viral load protection = 0.8 (high immune response)
• Day 20: Viral load protection = 1.0 (maximum protection)

They can of course be adjusted later for ExposureType, AgeGroup and VirusVariant.

[DavidKerkmann]

We agreed to leave out a common initialization in the simulations for the HighViralLoadProtectionFactor, as there was no fitting literature. In the future, we might have to review this, perhaps in combination with the other ProtectionFactors.

[DavidKerkmann]

Add a default constructor that initializes the function member to the result of get_default(). This ensures that the function is always properly initialized.
An operator() is defined to allow instances of HighViralLoadProtectionFactor to be called like a function. This operator forwards the call to the function member.
The function member is initialized in the constructor, ensuring that it always holds a valid callable.

This is from the changes on top of this PR. Is this still up-to-date? @khoanguyen-dev

[khoanguyen-dev]

Add a default constructor that initializes the function member to the result of get_default(). This ensures that the function is always properly initialized.
An operator() is defined to allow instances of HighViralLoadProtectionFactor to be called like a function. This operator forwards the call to the function member.
The function member is initialized in the constructor, ensuring that it always holds a valid callable.

This is from the changes on top of this PR. Is this still up-to-date? @khoanguyen-dev

I updated the description. It is now a simple extension for this parameter.

[DavidKerkmann]

I noticed that the pybinds were unchanged, perhaps this is a mistake from an earlier merge? I am confused why this was there in the first place, as it has nothing to do with this issue, but is also not on the main. What happened here?

[reneSchm]

Why do we use ExposureType and TimePoint as pair for e.g. latest_exposure in the Infection ctor or the tests? I think using latest_exposure_time/_type as separate values is easier than using .first/,second everywhere, right?

Staying with the changes made in this branch, they look good to me, though there are merge conflicts with the newly merged #1072 (sorry) that have to be resolved before I can approve this.

[DavidKerkmann]

Why do we use ExposureType and TimePoint as pair for e.g. latest_exposure in the Infection ctor or the tests? I think using latest_exposure_time/_type as separate values is easier than using .first/,second everywhere, right?

I think this got introduced when the function std::pair<ExposureType, TimePoint> Person::get_latest_protection() const got introduced, where the coupling was made and hence has been used like this everywhere. I agree separate value would be easier, but then we have to come up with something for this function, too. Any ideas?

[reneSchm]

I think this got introduced when the function std::pair<ExposureType, TimePoint> Person::get_latest_protection() const got introduced, where the coupling was made and hence has been used like this everywhere. I agree separate value would be easier, but then we have to come up with something for this function, too. Any ideas?

That makes sense. In that case, I think we could either split up the variables immediately after the function call, or we can add (yet another) simple struct for the protection. That way, we probably better preserve that the ExposureType is not used as (a person being) "exposed by", but rather as "protected from exposure by".

[reneSchm]

After the merge and additional renaming of Vaccine/ExposureType this is a very nice improvement.