Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add quarantine state #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

brushtyler
Copy link
Collaborator

If a symptomatic person has the app installed, all the people having the app which had contacts with him are quarantined as well.

Giuseppe Sucameli added 2 commits May 2, 2020 08:33
Add quarantine state so symptomatic people can be isolated,
revise prevention app simulation to quarantine people instead of stop them moving.

If a symptomatic person has the app installed, all the people having the app
which had contacts with him are quarantined as well.
Copy link

@Magicianred Magicianred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

La funzione dichiarata nella descrizione è implementata bene a livello di codice e di logica, nulla da dire. Ben fatto.
Bisogna aumentare la minor versione sul package.json

A livello di algoritmo, paragonato alla precedente versione, il risultato cambia (mi sembra relativamente in peggio), ma per questo ci vorrebbe un Data Scientist che convalidi il modello ed esula dalla Review.

@@ -25,16 +27,19 @@ export class Ball {
this.hasCollision = true
this.survivor = false
this.hasAppInstalled = hasAppInstalled
this.contacts = []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potresti auitarmi a capire contacts? Da quello che visto dal codice sarebbe un peopleEnteredInContact, corretto?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, contact list like contact-tracing. It's the list of other balls this ball came in touch while it's sick.

this.state = STATES.recovered
RUN.results[STATES.infected]--
RUN.results[oldState]--
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Puo' essere un valore diverso da infected?

Copy link
Collaborator Author

@brushtyler brushtyler May 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

infected or quarantine (see the if block above).
Changing the ball state to quarantine doesn't affect the time a person remains sick.

@brushtyler
Copy link
Collaborator Author

brushtyler commented May 3, 2020

@Magicianred

A livello di algoritmo, paragonato alla precedente versione, il risultato cambia (mi sembra relativamente in peggio), ma per questo ci vorrebbe un Data Scientist che convalidi il modello ed esula dalla Review.

I made some double-checks on it.
The previous version had a bug (i.e. a sick doesn't infect an healthy person if both have the app installed) so the previous results were completely wrong. To make a comparison we must fix the previous version and retry.
In addition, with current values of speed and timings there's high probability that when a person feels sick (i.e. the disease become visible) there are lots of infected people. We are working to make all to them configurable options, to improve the model tunability. I'm quite sure that using the right values the model can better explain the real world.

@dcremonini
Copy link

Please let's keep all the issues and PR requests in English to allow everybody to join our effor.

@brushtyler
Copy link
Collaborator Author

Since this PR has been partially overtaken by PR #2 (merged), I would suggest to close it and file an issue to analyze if the "quarantine" state gives any improvement over "incubating" + "sick".
Opinions?

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

Successfully merging this pull request may close these issues.

4 participants