Skip to content
This repository has been archived by the owner on Jun 23, 2023. It is now read-only.

Subtract mu (-0.3) from number of days as stated in equation 3 #6

Closed
t-chappell opened this issue Sep 24, 2020 · 8 comments
Closed

Comments

@t-chappell
Copy link

t-chappell commented Sep 24, 2020

https://github.com/nhsx/covid-19-app-android-ag-public/blob/01f790a0ebefe20ba6ff7925e56dfd88911741bf/app/src/main/java/uk/nhs/nhsx/covid19/android/app/exposure/encounter/InfectiousnessFactorCalculator.kt#L12-L17

Changes to this function needed for results to be inline with the equation.

  1. Make daysFromOnset a decimal/fractional data type for better accuracy of the duration part and use the correct start and end timestamps.
  2. Add 0.3 to daysFromOnset as stated in the equation,

val step1 = (daysFromOnset + 0.3) / sigma

@arcayn
Copy link

arcayn commented Sep 24, 2020

The distribution mean mu0 = -0.3, and the mean is being subtracted from the numerator. Therefore, if this correction is to be made, I believe it should be to add 0.3, rather than to subtract it.

@t-chappell
Copy link
Author

Not sure I follow, could be I'm missing something but the equation expressed in one line is:

exp( -0.5 * ( ( days::decimal - 0.3 ) / 2.75 ) ^ 2 )

What is currently in use:

exp( -0.5 * ( days::integer / 2.75 ) ^ 2 )

Below is equation 3 from the link in the comment above this function:

image

@t-chappell
Copy link
Author

ah yes, - - 0.3, updated issue

@t-chappell t-chappell changed the title Deduct 0.3 from number of days as stated in equation 3 Add 0.3 to number of days as stated in equation 3 Sep 24, 2020
@mezpahlan
Copy link

Apologies now I may not be understanding correctly..... but I think what we want is a mix of the original and the updated title 😝 . I realise there's a double negative thing going on but if you're looking at the equation and expecting the code to match I'd expect to see a minus in code not an addition.

So I think we should add mu as a constant in its negative form.

companion object {
  private const val sigma = 2.75
  private const val mu = -0.3 // Note the negative value here.
}

And then update the calculation like so:

// Calculated using equation 3 from the paper at https://arxiv.org/pdf/2005.11057.pdf
fun infectiousnessFactor(daysFromOnset: Int): Double {
  val step1 = (daysFromOnset - mu) / sigma
  val step2 = step1.pow(2)
  val step3 = -0.5 * step2
  return exp(step3)
}

I'm pretty sure the underlying language will understand the double negative and at least this way comparing equation 3 and the code is slightly more straight forward.

Apologies if this is off on the wrong direction. Either way, looks like IOS is using the same calculation.

@t-chappell t-chappell changed the title Add 0.3 to number of days as stated in equation 3 Subtract mu (-0.3) from number of days as stated in equation 3 Sep 27, 2020
@t-chappell
Copy link
Author

Ah yeah you're right lol, no worries. Will update again.

Yeah the new constant should be added, but also the "daysFromOnset: Int" should be a decimal/float data type or casted before hand to work with decimal numbers.

Have had a look through other parts and seems everything is in whole number of days with a max of 7, seems odd.

@arcayn
Copy link

arcayn commented Sep 27, 2020

@mezpahlan introducing another constant is almost certainly the sane way to go about this, I was more just concerned with the fact that a constant wasn't being introduced and the calculation was being made less accurate as a result

@t-chappell
Copy link
Author

any updates?

@nhs-covid19
Copy link
Collaborator

Thanks for your interest in the NHS Covid-19 project. I have asked one of the GAEN API specialists on the team to answer your query.

They say: We use the transmissionRiskLevel field on the key to represent daysFromOnset, however GAEN API limits us to only being able to use the values 0-7 which means we can't use fractional or negative values.
Because of not being able to send negative values, we send the absolute value of the difference in days. This abs(t) causes the curve to have a different shape and we decided that removing the mu parameter gave us a curve which was a closer approximation to the ideal
The blue curve is the ideal curve, in red is if we included mu=-0.3
mu is -0 3

Again blue curve is ideal, red is the actual equation we use
equation we use

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants