ecmwf / ecpoint-calibrate

Interactive GUI (developed in Python) for calibration and conditional verification of numerical weather prediction model outputs.
GNU General Public License v3.0
21 stars 8 forks source link

When we do not divide a variable in different branches, we want to see see the node with the values -inf<VAR<inf #159

Closed EstiGascon closed 3 years ago

EstiGascon commented 3 years ago

During the decision tree preparation process, sometimes we do not divide a parameter into different branches for a specific branch of the previous variable. If we see in the plot attached, we have 3 variables in this order: LSM, SDFOR and SR24. For the branch of LSM<0.5, we do not want to divide the second variable into branches, so we will have only 1 branch with the values -inf<SDFOR<inf. Then, we divide the branch again with the following variable "SR24h". However, in the plot, we cannot see the vertical straight line of 1 branch coming from LSM <0.5 to the SDFOR variable (so the node with the SDFOR variable does not exist). This is a bit confusing because now we have SDFOR for LSM<0.5 and SR24h for LSM>0.5 at the same horizontal level in this plot, so it can confuse the user to know if he/she is working with the second parameter or the third. Even if the final weather type is correct, it would be more helpful to have the corresponding SDFOR node with the interval -inf to inf.

Bug_tree

FatimaPillosu commented 3 years ago

Hi @onyb, I think what Esti is suggesting is the way we use to plot the predictors that had no breakpoints. If I remember correctly, we decided instead for this current option (not plotting them, and instead plot the nodes with different colours) to keep the DT more compact. Speaking with @AugustinVintzileos yesterday gives me the impression that he is ok with the current way of displaying the nodes, and actually even Esti might be happy with this version. @onyb, could you advise how long would it take you to implement this change? In case it is too long, I would be inclined to move it to a "nice to have", as this is not a problem that would block the work of Esti and Augustin, and if eventually, it takes too much time of your time, I would be inclined to leave as it is now and address other bugs.

onyb commented 3 years ago

TODO: we need to hide the nodes with thresholds same as the range, in case of circular variable.

onyb commented 3 years ago

Fixed in v0.28.0.