ARPA-SIMC / elaboradar

Library and tools to handle weather radar images and data (ODIMH5 and SP20)
GNU General Public License v2.0
3 stars 1 forks source link

Gestione default parametro in input #23

Closed brancomat closed 1 year ago

brancomat commented 1 year ago

in https://github.com/ARPA-SIMC/elaboradar/blob/f6096329731939dbdc36d98ef55164fee9660a3a/src/cum_bac_clparser.cpp#L86 è stata aggiunta la gestione di un parametro in ingresso (una directory). Se non viene specificato, dovrebbe puntare alla directory $(datarootdir)/$(PACKAGE) (vedi https://github.com/ARPA-SIMC/elaboradar/blob/master/dati/Makefile.am) che poi post-installazione rpm diventa /usr/share/elaboradar. Ci sono soluzioni più eleganti che hardcodare quest'ultimo path?

(il path specificato ora /home/ccardinali... al momento è un placeholder da sostituire)

brancomat commented 1 year ago

Aggiunta: il problema riguarda anche anche in altri due file che hanno default hardcoded: https://github.com/ARPA-SIMC/elaboradar/blob/f6096329731939dbdc36d98ef55164fee9660a3a/src/stat_CleanID.cpp#L30

https://github.com/ARPA-SIMC/elaboradar/blob/f6096329731939dbdc36d98ef55164fee9660a3a/src/RunCleanID.cpp#L28

spanezz commented 1 year ago

Ho aggiunto agli autotools la definizione della costante FUZZY_PATH a '${datadir}'"/$PACKAGE".

Ho cambiato src/cum_bac_clparser.cpp per usarla.

Ho cambiato src/stat_CleanID.cpp per usarla, riorganizzando il parsing della linea di comando per usare TCLAP come altrove in elaboradar.

Ho notato che in stat_CleanID.cpp, il parametro del volume di output non mi sembra fosse usato da nessuna parte.

Ho cambiato anche src/RunCleanID.cpp per usare FUZZY_PATH, e ho riorganizzato il parsing della linea di comando come con stat_cleanID.

Quello che purtroppo non ho potuto fare è testare se questi comandi fanno quello che devono una volta che compilano, ed è un compito che devo lasciare a voi.

Ho notato compilando che il compilatore emette warning abbastanza importanti a cui forse è meglio dare un'occhiata, tipo questo:

../radarelab/volume.h: In instantiation of ‘radarelab::PolarScan<T>& radarelab::volume::Scans<T>::append_scan(unsigned int, unsigned int, double, double) [with T = unsigned char]’:
stat_CleanID.cpp:82:35:   required from here
../radarelab/volume.h:340:25: warning: overflow in conversion from ‘double’ to ‘unsigned char’ changes value from ‘-1.9686274509803923e+1’ to ‘0’ [-Woverflow]
  340 |         this->push_back(PolarScan<T>(beam_count, beam_size));
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

A voi la palla, direi

chiara-arpae commented 1 year ago

Ciao! Abbiamo risolto il problema del warning indicato: le modifiche sono state pushate e riguardano l'aggiunta di un parametro opzionale alla funzione append_scan della classe PolarScan, il quale viene settato a 0 nel caso dell'oggetto full_volume_cleanID, altrimenti il valore è quello di default.

Tuttavia sono emersi dei problemi, legati al fatto che la macchina su cui deve girare operativamente elaboradar ha distribuzione linux centos, mentre le macchine su cui abbiamo sviluppato hanno distribuzione fedora. Per completezza specifico che sulla macchina centos, su cui stiamo facendo i test preoperativi, la versione del compilatore g++ è g++ (GCC) 8.5.0 20210514 (Red Hat 8.5.0-16) mentre sulla macchina fedora usata per lo sviluppo è g++ (GCC) 12.2.1 20220819 (Red Hat 12.2.1-2)

Detto questo, l'output del RunCleanID.cpp è molto diverso se giriamo su macchina-fedora o su macchina-centos ( e su macchina-centos è molto peggiore). Tali differenze dell'output non si avevano prima del tuo ultimo commit ( https://github.com/ARPA-SIMC/elaboradar/issues/23 ) sul passaggio di parametro, ma non capiamo come possa dipendere da queste modifiche visto che, da quel che capiamo, riguardano appunto solo il passaggio di parametro.

Inoltre, facendo delle stampe, ci siamo accorte che su centos, a differenza di fedora, la variabile check_undetect, dichiarata nel namespace in (namespace radarelab) in elabora_volume.h resta settata al valore 'true' passato in input alla TextureVD() nel main di RunCleanID.cpp, anche quando viene poi invocata EvaluateClassID() nel main.

EvalutateClassID(), definita in radarelab/algo/cleaner.cpp calcola altre Texture tramite la funzione TextureSD(). TextureSD prende in input anche il parametro force_check_undetect,il cui valore viene assegnato a check_undetect, tramite un passaggio by value. In EvaluateClassID() ci sono quattro invocazioni di TextureSD(), a cui viene passato force_check_undetect=false e force_check_undetect=true a seconda della quantità calcolata. Tuttavia il valore stampato di check_undetect a questo livello (la stampa era stata inserita nella funzione good()) risulta sempre true, cioè il valore che gli è stato assegnato nel main quando è stata invocata TextureVD(). Questo avviene solo su centos, mentre su fedora il valore di check_undetect risulta true quando viene invocata TextureVD() nel main di RunCleanID.cpp, mentre risulta false per tutte le invocazioni di TextureSD() in EvaluateClassID(), perchè viene passato 'false' al parametro force_check_undetect di TextureSD() alla prima invocazione e di nuovo, resta settato a false anche per quelle successive, all'interno di EvaluateClassID(). Su centos si ottiene la stessa situazione solo forzando il valore di check_undetect nel main di RunCleanID.cpp dopo l'invocazione di TextureVD() e prima dell'invocazione di EvaluateClassID() (intendo che prima di chiamare EvaluateClassID(), aggiungiamo una riga di codice 'check_undetect=false;'). Solo con questa forzatura risulta check_undetect=false anche in tutte le invocazioni di TextureSD() in EvaluateClassID() su centos.

Cioè la variabile check_undetect, dichiarata nel namespace radarelab, in elabora_volume.h, resta settata al valore passato nel main di RunCleanID.cpp se giriamo su centos. Su fedora invece il passaggio del parametro by value avviene correttamente nella prima chiamata di TextureSD() all'interno di EvaluateClassID(). Questo crea differenze notevoli nell'output.

Questa forzatura è una soluzione-tampone, ma forse si può risolvere in modo migliore se dipende dalla versione del compilatore o da come è stata definita questa variabile check_undetect. Che ne pensi?

Mi scuso per la prolissità, ma spero di aver messo in luce in modo chiaro i vari step seguiti e i problemi emersi. Ciao!! Chiara

chiara-arpae commented 1 year ago

Se può esserti utile ti alleghiamo

Ti scriviamo la riga di lancio : ./src/RunCleanID 2023-01-23-11-00-00.itgat.PVOL.0.h5 2023-01-23-11-00-00.itgat.PVOL.0.h5 GAT -U -F ./dati/ > logc.txt

Si notano le differenze tra i due files di output nel valore stampato dopo 'check' , che corrisponde al valore di check_undetect. Se ritieni opportuno, siamo disponibili per sentirci via meet. file_modificati.zip

spanezz commented 1 year ago

Direi che il problema sia che questa riga, in un header, non fa quello che sembra:

// In radarelab/algo/elabora_volume.h
namespace {bool check_undetect=false;}

Siccome check_undetect è definita in un namespace privato, il risultato è che non c'è un solo check_undetect, ma c'è un check_undetect diverso in ogni file in cui viene incluso elabora_volume.h.

In 4968461e07651f71631bd6b74924fb4ac87d47bc ho fatto push di una modifica che sposta check_undetect in un file .cpp, rendendolo unico, e mantenendone nell'header solo il riferimento.

Ho provato e con questa modifica il valore di check_undetect rimane deterministico in centos8 e fedora36

chiara-arpae commented 1 year ago

grazie mille! Confermo, per cpmpletezza, che i prodotti di ZLR e quelli corretti con VPR sono quelli attesi sia in fedora che in centos.