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

Added extra features to Climatic > Model > Extremes dialog #7721

Merged
merged 23 commits into from Nov 24, 2022

Conversation

Tula1
Copy link
Contributor

@Tula1 Tula1 commented Aug 3, 2022

@rdstern, @lloyddewit, @N-thony, @lilyclements This PR does not fix any issue but is part of my work.

I have been working with @Vitalis95 and @Ivanluv on improving the extremes dialog. These are the improvements:

i) Under the Bayesian fit method, prior parameters, Initial values and number of iterations was added to improve the model.
ii) We added the confidence intervals tab that outputs the parameter and return level, with corresponding return periods,
confidence intervals.
iii) We combined the fit method and display options sub dialogs into one sub dialog (Extremes Method) with separate tabs
ie fit method, Display Options and Confidence Interval to make it easier for the user.

@lloyddewit we have changed the Fitting Options tab in the main dialog to Options. Could you update this in the database to avoid merge conflicts when executed from my side.

@lloyddewit
Copy link
Contributor

@Tula1 Great to see you contributing to R-Instat. Welcome!

@lloyddewit we have changed the Fitting Options tab in the main dialog to Options. Could you update this in the database to avoid merge conflicts when executed from my side.

Done in PR #7724

@Tula1
Copy link
Contributor Author

Tula1 commented Sep 9, 2022

@rdstern , Please have a look.

@rdstern
Copy link
Collaborator

rdstern commented Sep 10, 2022

@Tula1 that is a considerable improvement. It will be good for you to explain, ready for testers, what you have done to test the new dialogue, before expecting others to check. This to include which data you used, and what models you fitted.
a) I like the new Options sub-dialogue. It is a good improvement. Please tidy the bottom of the sub-dialogue, so the controls are centred and there isn't that large space.
b) I tested with the dodoma data (i.e. a single site). I got the annual daily rainfall maximum which I used for the testing.

c) I tried the Bayesian fitting and it seems to work. I don't quite understand all the Bayesian options, so didn't change from the defaults that you supplied. Could you please give some descriptive details so I know what I can change. (I was fitting a gev.) I would like to alter aspects of the prior.

d) I then added the year so I am fitting a non-stationary model. It didn't work. I then found that the original didn't work either, and that's because it doesn't like missing values that are different for the dependent and independent variables. (We do need to be able to filter out missing values in the data.) This was easy in this case, as it was just the last 2 years. So I deleted them.

e) Then I tried again. I found the Bayesian doesn't work now. Possibly something needs to change in the priors. The ordinary MLE works fine. Please could you check this.

@Tula1
Copy link
Contributor Author

Tula1 commented Oct 14, 2022

@rdstern The issues raised above have now been sorted. Attached is a description of how the added parameters in the dialogs work. I have used the Dodoma Dataset to test the dialogs.

Please have a look at it.

Testing the Extremes Dialogs..docx

@rdstern
Copy link
Collaborator

rdstern commented Oct 14, 2022

great - thanks

@lloyddewit
Copy link
Contributor

great - thanks

@rdstern Will you approve or should @Tula1 still make changes? thanks

@N-thony
Copy link
Collaborator

N-thony commented Oct 24, 2022

great - thanks

@rdstern Will you approve or should @Tula1 still make changes? thanks

@rdstern would you like to approve or they are still do here?

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@lloyddewit and @N-thony I see I sort of "dropped the ball" on this one!). It is now a considerable improvement on the existing dialogue and it would be good to have it merged for the next release. I hope the code is ok or that either of you could make it ok, given @Tula1 has finished her internship.
I have copied the main points from here into an issue, so we can continue with the discussion by using the dialogue, once this is merged.

Copy link
Contributor

@lloyddewit lloyddewit left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@lloyddewit lloyddewit changed the title Improving the Climatic > Model > Extremes dialog Added extra features to Climatic > Model > Extremes dialog Nov 24, 2022
@lloyddewit lloyddewit merged commit 4057a6f into IDEMSInternational:master Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants