Update distributions.py

[Davide-Ruffini]

<Multinomial added

[EdoLu]

Ciao @Davide-Ruffini , @gpitt71 :
Di seguito alcuni miei commenti:

• Manca la nuova multinomiale nel DIST_DICT contenuto in config.py.
• Esiste nella distribuzione sottostante di scipy un metodo simil "moment"? E' coerente con skewness e kurtosis che abbiamo aggiunto, i.e. funziona allo stesso modo? Vale la pena aggiungere "var" o esiste già?
• Metodi par_deductible_adjuster e par_deductible_reverter: rispetto al caso univariato nu potrebbe (e tipicamente dovrebbe) essere un array di dimensione pari al parametro p delle probabilità. Farei alcuni assert e controlli iniziali che le dimensioni di p e nu siano compatibili, inoltre avete controllato che le funzioni si comportano secondo le attese anche in questo caso?
• I metodi ereditati da _DiscreteDistribution (e da _Distribution) funzionano?

[EdoLu]

Commento aggiuntivo meno importante: di solito le pull request (PR) riguardano più contenuti e non solo un singolo commit.
Questa è la prima volta, e quindi ha senso farlo come prova, però in futuro farei una PR dopo che si hanno implementato più aspetti, ad es. una serie o una famiglia intera di distirbuzioni multivariate.

A presto

[gpitt71]

Ciao @Davide-Ruffini @EdoLu,

@Davide-Ruffini grazie ancora per il lavoro che stai facendo. Sembra un buon inizio!

Di seguito i miei commenti:

• Metodo pgf. Docustring. La documentazione non sembra corretta. La parametrizzazione abk è solo per univariate o mi perdo qualcosa?
• Metodi par_deductible_adjuster, par_deductible_reverter. Non sono ancora implementati perchè non ancora inclusi aspetti di LossModelling. Corretto? Vale la pena che ritorinino NULL o che non ci siano proprio?
• Skewness e curtosi non dovrebbero ritornare i risultati delle binomiali marginali? Altrimenti manca forse un po' di coerenza matematica. Dovrebbero forse avere un parametro: marginal_id o cose del genere che l'utente può usare per richiedere risultati per una specifica marginale.

Eventuali aspetti aggiunti dovrebbero essere documentati.

Aspetto un vostro feedback,
Ciao,
Gabriele

[Davide-Ruffini]

Test.txt

Ciao a tutti,

Inizio con una breve descrizione:

1. I metodi della funzione multinomial di scipy sono stati replicati tramite wrapper (pmf, logpmf, rvs, entropy e cov).
2. I metodi implementati in _DiscreteDistribution e _Distribution che non sono presenti nella funzione di scipy sono stati inseriti all'interno della nuova funzione utilizzando una nuova classe _MultDiscreteDistribution, definita come Child class di _DiscreteDistribution. Il concetto attualmente applicato è quello di utilizzare gli stessi metodi già definiti sulle singole marginali che compongono la nostra multivariata (es: se applico la funzione curtosi sulla Multinomiale ottengo cosi la curtosi delle singole binomiali)

Procedo con ordine:

@EdoLu

A - Ho aggiunto la funzione al dizionario DIST_DICT contenuto in config.py. (in fase di prima Pull ho avuto qualche problema a capire come modificare più script con la stessa richiesta, ora dovrei aver capito)
B - Da quel che ho visto per la funzione multinomiale i metodi della multinomiale di scipy non sono molti, ovvero: pmf, logpmf, rvs, entropy e cov - questi sono stati recepiti nella funzione e dai test che ho fatto restituiscono gli stessi risultati. Non ho visto un metodo simil moment in questo contesto, se hai visto qualcosa del genere fammi sapere.
C - Metodi par_deductible_adjuster e par_deductible_reverter: Li ho rimossi del tutto per evitare confusione - sostanzialmente non sono ancora implementati. Potrebbe aver senso inserirli nel momento in cui si aggiorna anche lossmodel, cosi da poter testare il risultato? Anche in quel caso avrò sicuramente bisogno del vostro supporto per definire l'output desiderato
D - vedi punto 2 all'inizio

@gpitt71

a - Potrei aver creato confusione lasciando funzioni con metodi non implementati (probabilmente la mia idea era quella di ricevere un vostro feedback per capire come gestirli), ora il metodo pgf è stato rimosso completamente. Se poi ci sono altri modi in cui lo si vuole implementare fammi sapere (il tema della parametrizzazione abk mi confonde un po')
b - vedi punto C nelle risposte a Edoardo
c - skewness e curtosi ora restituiscono i risultati delle binomiali marginali (utilizzando la classe _MultDiscreteDistribution). La prima risposta che mi viene in mente sul tema "marginal_id" è che potrebbe essere rappresentato in modo naturale dalla posizione della marginale - la prima curtosi restituita sarà quella della prima marginale e cosi via.

In tutto ciò quello che mi chiedo è: nel momento in cui si utilizza una multivariata per fare pricing - è più utile sapere i valori di skewness delle singole marginali, o sarebbe più utile avere una misura che mi descriva l'asimmetria complessiva della distribuzione multivariata che utilizzo?

Fatemi sapere cosa ne pensate - se utile possiamo anche parlarci a voce in una chiamata
Ciao,
Davide

[EdoLu]

CIao @Davide-Ruffini,

grazie del lavoro svolto e dei commenti.

Ti chiederei di apportare le seguenti modifiche:

• La stringa che rappresenta il nome della distribuzione multinomiale ritornato dalla proprietà "name" andrebbe con la iniziale minuscola. Quindi, anche in DIST_DICT di config.py, "Multinomial" deve essere con la "m" minuscola, semplicemente per essere coerenti con le altre distribuzioni.
• Molto elegantemente pythonico il modo in cui hai implementato la classe _MultDiscreteDistribution, bravo :). Tuttavia mi chiedo se è corretto e serve avere quel tipo di classe che funziona in quel modo. Mi devo prendere qualche tempo per pensarci, @gpitt71 cosa ne pensi? Forse dovremmo fare una chiamata e discuterne.

In merito al tuo commento:
In tutto ciò quello che mi chiedo è: nel momento in cui si utilizza una multivariata per fare pricing - è più utile sapere i valori di skewness delle singole marginali, o sarebbe più utile avere una misura che mi descriva l'asimmetria complessiva della distribuzione multivariata che utilizzo?
Secondo me no. Non credo nemmeno esista un concetto comunemente condiviso e standard di momenti, come la skewness, in ambito multivariato. Esistono dei lavori nella letteratura che sviluppano questi momenti per le distribuzioni multivariate, ma a ora non penso siano utili ai nostri fini. Quindi tutte queste grandezze hanno senso per noi solo a livello marginale. Diverso è il discorso per le analoghe misure della distribuzione somma (o comunque aggregata), ma adesso non è rilevante.

@gpitt71: secondo me varrebbe la pena creare un nuovo branch dal main/master branch, che chiamiamo ad esempio "developer", dove @Davide-Ruffini (o un qualsiasi sviluppatore, noi compresi) indirizza le Pull Request. Non é il massimo della buona prassi mandare le PR direttamente nel main/master branch. Prima andrebbero testate.

Edo

[Davide-Ruffini]

Ciao @EdoLu

Grazie per i commenti, ho inserito messo le lettere minuscole come da indicazione.
Se occorresse un confronto tramite chiamata o qualsiasi cosa fatemi sapere!

Davide

[gpitt71]

Ciao @EdoLu @Davide-Ruffini,

@Davide-Ruffini , grazie per il contributo e ottimo lavoro!

In risposta alle vostre domande:

• @Davide-Ruffini, faccio riferimento a quanto scritto da Edoardo. Ok per quanto scrivi.
• @EdoLu, tema della classe Parent. Se @Davide-Ruffini non è stanco di noi, possiamo decidere l'orientamento del progetto e vedere cosa funziona/funzionerà meglio a livello di struttura finale. Penso che un eventuale refactoring possa essere rapido.

Questa mattina ho fatto alcuni test sul codice e scrivo qui sotto i miei dubbi. In particolare, ho testato tutti i nuovi metodi e ho le seguenti domande:

• pmf/logpmf/cdf: esattamente cosa stiamo calcolando? Consideriamo di avere tre marginali X1 X2 X3 ed una size n=ntot, con probabilità p1 p2 p3. Con riferimento all'implementazione in R, la funzione dmultinom ritorna P(X1=n1,X2=n2, X3=ntot-n1-n2) e non ha forse moltissimo senso avere un metodo cdf? Come vedrete sotto loro implementano solo la probability mass function. Inoltre, forse dovremmo avere un controllo che qualora l'utente mettesse dei valori che non sommano al parametro n ritorni un errore. Per capire meglio quanto scrivo, allego un esempio in R. Inoltre sembra che

> dmultinom(c(1,0,3),size=4, prob=c(.2,.4,.4))
[1] 0.0512
> dmultinom(c(1,1,3),size=4, prob=c(.2,.4,.4))
Error in dmultinom(c(1, 1, 3), size = 4, prob = c(0.2, 0.4, 0.4)) :  size != sum(x), i.e. one is wrong

Da quanto vedo nel codice per pmf sembra che non abbiamo il controllo menzionato sopra e implementato in R ma semplicemente qualora il risultato non sia possibile ritorniamo zero

• Quando le probabilità messe a parametro non sommano uno vengono standardizzate o serve che sommino 1?

• Cos'è entropy in contesto multivariato?

[Davide-Ruffini]

Ciao Gabri,
per punti:

1. Pmf / logpmf/ cdf: Buon punto. In effetti anche l’attuale funzione multinom di scipy non implementa il metodo cdf, cosi come dmultinom in R.
   Diciamo che al momento, se per pmf e logpmf utilizziamo gli stessi metodi di scipy, invece per la cdf restituiamo la cdf delle marginali – non credo sia sbagliato, ma potrebbe essere inutile… Dipende forse dagli utilizzi che avrà questa funzione per gli sviluppi futuri?

2. Al momento la struttura è la seguente: per i metodi implementati in scipy per la funzione multinom utilizziamo la loro stessa logica, pertanto ereditiamo anche gli stessi controlli (questo vale quindi per rvs, cov, entropy, pmf e logpmf), mentre gli altri metodi sono implementati sulle marginali, utilizzando i rispettivi controlli implementati in queste.
   Potremmo inserire un controllo a monte che restituisca un errore se le probabilità non sommano ad 1 o effettui una normalizzazione, ditemi pure voi cosa preferite!

3. Concetto di entropy in contesto multivariato: Una “non risposta” potrebbe essere che è quanto implementato su scipy, che guardando la fonte effettua il seguente calcolo:
   f(x) = -log(n!) - n * Σ(p_i * log(p_i)) + Σ(Σ(C(n, x) * p_i^x * (1-p_i)^(n-x) * log(x!)))
   Sembra quindi che abbiano aggiustato il concetto di entropia di modo da avere un senso in questo contesto.
   Se è un concetto non utile ai nostri fini o che non convince si può valutare la rimozione, fatemi sapere!

[gpitt71]

Ciao @Davide-Ruffini,

ottimo! Grazie per i chiarimenti. Tutto fila.

Unico punto, secondo me vale la pena togliere il metodo cdf. A maggior ragione se ritorniamo le cdf marginali. Prendi quanto segue come un'opinione, da un punto di vista puramente applicativo se siamo espliciti su cosa stiamo facendo potrebbe essere ok quello che hai fatto anche se non mi piace moltissimo.

Nel dettaglio, vorrei che il pacchetto fosse sempre preciso dal punto di vista tecnico e nell'implementazione dovrebbe valere la relazione con la pdf/pmf che trovi qui: https://en.wikipedia.org/wiki/Cumulative_distribution_function, sezione multivariate case . @EdoLu cosa ne pensi?

Ciao e grazie ancora!

G

[Davide-Ruffini]

Ciao a tutti,

Ho inserito i seguenti aggiornamenti discussi:

1 - CDF e LogCDF rimosse per la multinomiale (e per le multivariate in generale, visto che i metodi erano applicati tramite quella classe sulle univariate e la cosa non ci sembrava aver senso)
2 - Metodo Var aggiunto alla multinomiale in modo consistente con Cov - quindi con Var andiamo ora a restituire la diagonale della matrice di Cov della multinomiale
3 - ho aggiunto l'errore implementato nella funzione di "R". Vista la meccanica della funzione di Scipy questo problema si manifesta nei metodi pmf e logpmf, pertanto ho aggiunto il controllo a quel livello.
Fatemi sapere se questa soluzione ha senso secondo voi e funziona correttamente. Di seguito due esempi di output

Fatemi sapere! @gpitt71 @EdoLu

[gpitt71]

Ciao Davide, mi sembra che sia quasi tutto ok!

Ho notato che abbiamo ancora i metodi legato alla survival function (1-CDF) che andrebbero rimossi in quanto collegati alla CDF (sf, isf, logsf). Per il resto direi grande lavoro!

G

[Davide-Ruffini]

Giusto! Sono attualmente senza PC, qualche giorno che rientro a casa e sistemiamo anche quello
Davide

[Davide-Ruffini]

Fatto!

[gpitt71]

Ciao Davide. Grazie molte per il lavoro che hai fatto! Ho effettuato il merge.