ARPA-SIMC / dballe

Fast on-disk database for meteorological observed and forecast data.
Other
19 stars 6 forks source link

importing value outside the range #241

Closed pat1 closed 3 years ago

pat1 commented 3 years ago

Importando dati da json a volte ottengo errori di questo tipo: Cannot parse JSON message #5406: Value -50 is outside the range [-20,1048554] for 013013 (TOTAL SNOW DEPTH) at offset 1384461.

Se non mi ricordo male durante l'importazione da bufr/conversione di tempate bufr avevamo previsto la possibilità di importare comunque il dato assegnando il valore minimo/massimo del range aggiungendo un attributo.

Sarebbe possibile facendo seguito a una decisione del gruppo dati introdurre questa possibilità nei tools e nelle API per l'importazione da json?

spanezz commented 3 years ago

Penso di sí. Aggiungi tu il gruppo dati agli assegnatari del ticket?

Se decidete di andare su questa strada, puoi aggiungere al ticket un JSON che riproduce la cosa?

pat1 commented 3 years ago

mi sono espresso male, il gruppo dati si è già espresso all'unanimità. Allego un file di esempio. cae.zip

brancomat commented 3 years ago

Se non ricordo male siamo in attesa di un cambio di API sul branch issue239 collegato alla #239, visto che anche questa impatta sulle API si potrebbe approfittarne per mergiare entrambi i cambiamenti nel master e fare una nuova release con cambio di .so e tutto quanto

spanezz commented 3 years ago

Se non mi ricordo male durante l'importazione da bufr/conversione di tempate bufr avevamo previsto la possibilità di importare comunque il dato assegnando il valore minimo/massimo del range aggiungendo un attributo.

Ho cercato, per avere una specifica precisa su cosa fare nel caso JSON, ma non trovo codice che faccia questa cosa o ticket che la chiedano. Mi puoi trovare un riferimento?

In alternativa, se tutto il gruppo dati è d'accordo, trasformiamo questo ticket in una richiesta di gestire i dati fuori dominio, quando si importa qualunque formato, ai valori degli estremi del dominio.

Nel qual caso, lo volete come comportamento di default (per cui non sarà piú possibile in dballe avere un errore in caso di valore fuori dominio), o un comportamento da attivare solo in alcune circostanze? Nel qual caso, come preferite attivarlo/disattivarlo?

pat1 commented 3 years ago

In alternativa, se tutto il gruppo dati è d'accordo, trasformiamo questo ticket in una richiesta di gestire i dati fuori dominio, quando si importa qualunque formato, ai valori degli estremi del dominio.

il gruppo si è già espresso in senso generale; il gruppo è formato agnostico la richiesta l'ho formulata su json perchè mi pareva che da bufr fosse già fatta in questo modo (bufr non generici verso generici) e oltre a json non mi pareva ci fossero altri formati, ma forse mi son perso qualche cosa comunque, si, la issue si risolve a tutti i formati. Io terrei l'attuale comportamento come default e aggiungerei all'api la possibilità di specificare questo comportamento "tollerante" quando voluto. dove e come accetto proposte e suggerimenti

spanezz commented 3 years ago

Lo stato attuale direi che sia questo commit in wreport del 2012 che introduce la possibilità, quando ci sono valori fuori dominio, di metterli a unset

spanezz commented 3 years ago

Però è un'opzione disabilitata per default, e in dballe non trovo codice che la abiliti. Sembra sia usata solo in arkimet per la scansione dei BUFR

pat1 commented 3 years ago

secondo me c'è qualche cosa quando si converte da template e template perchè mi ricordo che in alcuni casi nel cambio di variabile B si aveva questo problema, ma forse mi sbaglio ?

edigiacomo commented 3 years ago

Ho assegnato la issue ai membri del gruppo dati presenti su github (@edigiacomo @pat1 @dcesari), in modo che possiamo fare da tramite con il resto del gruppo dati, se necessario (ma credo che l'implementazione non interessi loro).

Sono d'accordo con @pat1 sul fatto che la modifica debba essere retrocompatibile e quindi di attivare esplicitamente la trasformazione dei valori fuori range.

Gli scenari di importazione che conosco si limitano a importazione dei dati da bash e da Python. Visto che si vuole tenere il comportamento di default, si potrebbe anche delegare ad un modulo terzo la trasformazione a valle del dato, lasciando inalterati CLI e libreria Python di dballe.

Ad esempio, da bash si potrebbe passare da:

programma-che-produce-json | dbadb import

a

programma-che-produce-json | modifica-valori-fuori-scala [opzioni] | dbadb import

E in Python, invece di importare direttamente i dati dai messaggi con dballe.DB.import_messages si potrebbe avere un modulo che legge il JSON e lo converte in una lista di dballe.Message (in modo analogo a quanto fatto da dballe.Importer) ma modificando i valori fuori range.

from modulo_che_gestisce_dati_fuori_range import Importer
options = { ... }
importer = Importer("BUFR", **options)
with importer.from_file("test.bufr") as f:
    for msgs in f:
        ...

Le opzioni che mi vengono in mente per CLI/API sono:

Se non ci vedete di problemi, si potrebbe addirittura delegare questo comportamento a ARPA-SIMC/dba_qcfilter.

edigiacomo commented 3 years ago

Se non ci vedete di problemi, si potrebbe addirittura delegare questo comportamento a ARPA-SIMC/dba_qcfilter.

Mi sono espresso malissimo e ho dimenticato un pezzo: intendo dire che dba_qcfilter potrebbe essere esteso (e magari rinominato) oppure si può creare un ulteriore progetto (ma analogo) se vogliamo tenere separati i due argomenti.

pat1 commented 3 years ago

attenzione che esiste anche fortran e la libsim legge direttamente con le api dballe. E' vero che avere un tool separato rende la cosa più modulare, ma i range devono essere dedotti da dballe.txt e sinceramente mi parrebbe più semplice delegare questo lavoro a dballe. Tenete conto che questa "feature" deve essere anche distribuita anche ad esempio in ambito mistral per l'importazione dei dati

spanezz commented 3 years ago

Io posso aggiungere a wreport l'opzione, come ora fa ignore se richiesto per arkimet, il fare "clamp" agli estremi di dominio.

Posso aggiungere a dbadb import un'opzione --domain-errors={unset|clamp} che setta l'opzione corrispondente di wreport.

Posso aggiungere un parametro opzionale corrispondente alle funzioni di import dell'API Python, visto che Python lo supporta.

Non ho idea di come poter esporre un'opzione del genere nell'API Fortran, né se serva.

Detto questo, "qcfilter" suona comunque come un buon posto dove mettere preprocessatori che facciano pulizia di dati di questo tipo, che magari fan comodo al di là di dbadb.

edigiacomo commented 3 years ago

Per me va bene anche la proposta di @spanezz (sia l'introduzione della funzionalità in dballe che il clamp). Rimane solo la scelta dell'attributo: uno fisso oppure configurabile (ed eventualmente con un default)? Su quest'ultima parte non ho personalmente grosse preferenze nè necessità, quindi lascio la parola a @pat1 e @dcesari.

pat1 commented 3 years ago

riguardo al fortran anche pui accetta parametri oozionali quindi si può risolvere come per il python, ma lascerei il problema a una fase successiva di adeguamento. quindi dire proposta di @spanezz accettata. come attributo propongo di usare 033192 posto a zero, ossia la stessa cosa che fa il gross error check con soglie solitamente più stringenti.

spanezz commented 3 years ago

Se ci volete anche un'attributo, allora credo sia meglio lo script di preprocessing, che non la gestione a livello di wreport. Non mi piace l'idea di inchiavardare a livello di libreria di decoder del formato una strategia che appartiene piú a un livello di controllo qualità.

Al limite, in fase di implementazione di un qcfilter, se wreport non dà abbastanza possibilità di gestire questo tipo di anomalie, possiamo vedere come aggiungerle

dcesari commented 3 years ago

Dopo spiegazione di Pat1 a parte per i non comprendenti, confermo che per me va bene la proposta e sono anche d'accordo che non è compito di wreport aggiungere l'attributo, ma di un tool specifico.

pat1 commented 3 years ago

secondo me c'è qualche cosa quando si converte da template e template perchè mi ricordo che in alcuni casi nel cambio di variabile B si aveva questo problema, ma forse mi sbaglio ?

forse in bufr2netcdf ?

dcesari commented 3 years ago

Forse non tanto in bufr2netcdf stesso ma nella conversione preliminare di template con dbamsg convert [--bufr2netcdf-categories].

pat1 commented 3 years ago

comunque sorge una domanda: cosa succede ora, con la versione dballe attuale se si importa in dballe da un bufr non generico quindi con una tabella B non dballe.txt quindi ad esempio con una temperatura fuori range per la tabella dballe.txt ?

spanezz commented 3 years ago

Ho guardato al codice di --bufr2netcdf-categories e cambia solo categorie/sottocategorie dei messaggi, non i valori delle variabili.

spanezz commented 3 years ago

Ho implementato dbadb --domain-errors. L'opzione --domain-errors=unset va col wreport già pacchettizzato, e l'opzione --domain-errors=clamp richiede il wreport appena committato in master. dballe dovrebbe compilare correttamente con entrambi, attivando o disattivando 'clamp' se è presente o no.

Per eventuali sviluppi ulteriori, esporre queste funzioni alle API, o nuove API (se quelle attuali non bastano) per scrivere programmi di controllo qualità, fatemi sapere cosa serve nel caso concreto.

pat1 commented 3 years ago

ottimo. per arrivare a un secondo step e pacchettizzare dballe testarlo e distribuirlo anche a mistral mi servirebbe una api python per concretizzare la cosa anche in un pezzo di codice tipo questo: https://github.com/r-map/rmap/blob/8fc2b1a09c05f9052606f4fb12a25d8377017c7c/python/amqp2amqp_jsonline2bufrd#L62

spanezz commented 3 years ago

Ho aggiunto nella API Python al costruttore di dballe.Importer il parametro domain_errors="raise", che può essere settato a "unset" o "clamp".

Non ho al momento idee di come aggiungere questa funzionalità alla API Fortran senza rompere la ABI della libdballef.

Intanto ho fatto merge di tutto in master. Proporrei di chiudere qui questo ticket, e se c'è bisogno di questa funzionalità anche nella API Fortran, aprire un ticket in cui discutere i dettagli.

pat1 commented 3 years ago

aperta issue #244 Prima di chiudere mi piacerebbe capire cosa succedeva prima se si importava in dballe da un bufr non generico quindi con una tabella B non dballe.txt quindi con la possibilità di avere un valore fuori range per la tabella dballe.txt ?

spanezz commented 3 years ago

Prima, come ora se non si setta domain_errors, se cercava di leggere un valore fuori range sollevava un'eccezione, eccetto solo nel caso di arkimet che scansiona BUFR, nel qual caso un valore fuori range viene messo a unset

pat1 commented 3 years ago

passando la nuova versione dballe in preproduzione per Mistral ci siamo accorti che l'attributo non viene assegnato. Ossia 033192 non c'è e non è posto a zero. outofrange.zip

cat test_outrange.jsonl |python jsonl2bufr_nifi_2_py > test_outrange.bufr
brancomat commented 3 years ago

@pat1, magari sbaglio ma mi sembra che il comportamento del test case sia in linea con quanto deciso in questa issue:

pat1 commented 3 years ago

Abbiamo opinioni diverse su quanto sarebbe stato deciso in questa issue. Si può vedere quanto deciso dal gruppo dati e la richiesta dei dettagli implementativi. Il problema si pone in tutti i contesti in cui non c'è un QC sui dati, vedi Mistral e l'accoglienza dei dati CAE così come in tutte le situazioni in cui ci sono conversioni di formato e tutti i casi in cui si importano dati in formato json e l'uso dei tools libsim. Anche l'applicazione COSUDO fa affidamento sulla presenza di 033192. Concordo che sarebbe meglio scindere tra importazione dei dati e applicazione di un QC e quindi avere queste 3 possibilità: 1) importazione dei dati con gestione dell'errore fatta dall'utente 2) importazione dei dati con trasformazione dei fuori range 3) importazione dei dati con trasformazione dei fuori range e aggiunta attributo come primo gross error check del QC

033192 = 0 infatti si chiama gross error check perchè grossolano

Per fare il terzo punto indipendentemente da dballe bisognerebbe esportare da dballe tutti i valori di range per poter riassegnare su tale base 033192 = 0 a posteriori dopo l'importazione in modo asincrono (e oneroso) o forzare l'importazione su file per elaborare i dati in pipe e scrivere su DB. Una serie di valori di range gestiti in modo indipendente da dballe.txt creerebbe un casino nella gestione, dovendo tenere le due tabelle allineate e dovendone gestire la distribuzione in maniera sincrona. Il vero QC climatologico si occupa di assegnare gli altri valori di 033192 e eventualmente assegnare 033192 = 0 nei casi lo ritenga opportuno su criteri più stringenti del fuori range. Nei miei casi d'uso non ho mai il punto 2) e ho sempre il 3) Inoltre non assegnare l'attributo appesantisce l'eccezione fatta in questo trattamento dei dati che prevede che il dato non venga manipolato e modificato non dando nessuna segnalazione sul fatto che questa manipolazione sia avvenuta. Si potrebbe discutere se sia meglio utilizzare una flag differente specifica, ma ritengo che l'onere di gestione nella maggioranza dei casi e l'appesantimento nella gestione di un attributo specifico non ne valga la pena. La soluzione di questa issue comprendente l'attributo è stata proposta in riunioni di programmazione Mistral a cui credevo fossi presente tu e altri colleghi @brancomat. Soluzioni diverse prevedono la gestione degli estremi fuori range e la scrittura un applicati apposito e la sua distribuzione e una riformulazione nelle procedure Mistral e nella procedura di acquisizione dati da CAE e quelle preventivate da libsim con la aggiunta di parametri nelle API fortran. Aggiungo che la soluzione di questa issue è molto urgente.

spanezz commented 3 years ago

Mi rifiuto di aggiungere a wreport gestioni hardcoded di questo tipo: non è il livello di astrazione giusto.

Posso aggiungere a wreport un hook chiamato in caso di valori fuori dominio, che poi dballe può implementare settando i valori che vuole, o gli attributi che vuole presi dalla tabella B locale.

Che dite?

brancomat commented 3 years ago

Per me ok

edigiacomo commented 3 years ago

Benissimo

spanezz commented 3 years ago

Dovebbero ora esserci, a patto di compilare con wreport >= 3.29:

dcesari commented 3 years ago

Ottimo, però cito dall'help (non so cosa faccia in effetti il codice):

..."raise" (the default) raises an
exception. "unset" changes the value to be unset. "clamp"
changes the value to the nearest valid extreme of the
domain. "tag" unsets the value and sets attribute B33192=0

ma "tag" non dovrebbe invece fare come clamp+attributo, cioè "changes the value to the nearest valid extreme of the domain and sets attribute B33192=0"?

edigiacomo commented 3 years ago

Guardando il codice (https://github.com/ARPA-SIMC/wreport/blob/v3.29-1/wreport/var.cc#L410-L423 e https://github.com/ARPA-SIMC/dballe/blob/v8.16-1/dballe/msg/domain_errors.cc#L12-L25) mi pare consistente con la documentazione.

Nel convertitore da VM2 a BUFR ho risolto implementando un hook che fa clamp + set di B33192=0: https://github.com/ARPA-SIMC/meteo-vm2/commit/1e45c102ea2ca02ecce990400f4b9267f8f156df.

Faccio una PR per dballe.

spanezz commented 3 years ago

Cambiamolo in clamp+attributo, per me una vale l'altra. Il codice per il clamp è in wreport/var.cc

edigiacomo commented 3 years ago

Ok, faccio il merge della PR #250