Provide location parameter to KS test, address issue #39

[eastjames]

This pull request fixes the KS Test by providing the location (fixed) parameters and resolves issue #39.

[georgebv]

Thank you. A few requests:

• let's keep naming consistent and call the variable fixed_parameters
• please update docstring for the new parameter
• update tests

[eastjames]

Added a new commit to address these points

[codecov[bot]]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (b8cf576) 100.00% compared to head (0797aff) 100.00%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #55 +/- ## ========================================== Coverage 100.00% 100.00% ========================================== Files 16 16 Lines 784 106 -678 ========================================== - Hits 784 106 -678 ``` | [Impacted Files](https://app.codecov.io/gh/georgebv/pyextremes/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=George+Bocharov) | Coverage Δ | | |---|---|---| | [src/pyextremes/models/distribution.py](https://app.codecov.io/gh/georgebv/pyextremes/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=George+Bocharov#diff-c3JjL3B5ZXh0cmVtZXMvbW9kZWxzL2Rpc3RyaWJ1dGlvbi5weQ==) | `100.00% <ø> (ø)` | | | [src/pyextremes/models/model\_base.py](https://app.codecov.io/gh/georgebv/pyextremes/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=George+Bocharov#diff-c3JjL3B5ZXh0cmVtZXMvbW9kZWxzL21vZGVsX2Jhc2UucHk=) | `100.00% <ø> (ø)` | | | [src/pyextremes/models/model\_emcee.py](https://app.codecov.io/gh/georgebv/pyextremes/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=George+Bocharov#diff-c3JjL3B5ZXh0cmVtZXMvbW9kZWxzL21vZGVsX2VtY2VlLnB5) | `100.00% <ø> (ø)` | | | [src/pyextremes/\_\_init\_\_.py](https://app.codecov.io/gh/georgebv/pyextremes/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=George+Bocharov#diff-c3JjL3B5ZXh0cmVtZXMvX19pbml0X18ucHk=) | `100.00% <100.00%> (ø)` | | | [src/pyextremes/eva.py](https://app.codecov.io/gh/georgebv/pyextremes/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=George+Bocharov#diff-c3JjL3B5ZXh0cmVtZXMvZXZhLnB5) | `100.00% <100.00%> (ø)` | | | [src/pyextremes/extremes/block\_maxima.py](https://app.codecov.io/gh/georgebv/pyextremes/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=George+Bocharov#diff-c3JjL3B5ZXh0cmVtZXMvZXh0cmVtZXMvYmxvY2tfbWF4aW1hLnB5) | `100.00% <100.00%> (ø)` | | | [src/pyextremes/extremes/extremes.py](https://app.codecov.io/gh/georgebv/pyextremes/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=George+Bocharov#diff-c3JjL3B5ZXh0cmVtZXMvZXh0cmVtZXMvZXh0cmVtZXMucHk=) | `100.00% <100.00%> (ø)` | | | [src/pyextremes/extremes/peaks\_over\_threshold.py](https://app.codecov.io/gh/georgebv/pyextremes/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=George+Bocharov#diff-c3JjL3B5ZXh0cmVtZXMvZXh0cmVtZXMvcGVha3Nfb3Zlcl90aHJlc2hvbGQucHk=) | `100.00% <100.00%> (ø)` | | | [src/pyextremes/extremes/return\_periods.py](https://app.codecov.io/gh/georgebv/pyextremes/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=George+Bocharov#diff-c3JjL3B5ZXh0cmVtZXMvZXh0cmVtZXMvcmV0dXJuX3BlcmlvZHMucHk=) | `100.00% <100.00%> (ø)` | | | [src/pyextremes/models/models.py](https://app.codecov.io/gh/georgebv/pyextremes/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=George+Bocharov#diff-c3JjL3B5ZXh0cmVtZXMvbW9kZWxzL21vZGVscy5weQ==) | `100.00% <100.00%> (ø)` | | ... and [4 files with indirect coverage changes](https://app.codecov.io/gh/georgebv/pyextremes/pull/55/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=George+Bocharov)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[georgebv]

Thank you James. I have reviewed the PR and I realized that I previously misunderstood the problem. A fix still needs to be pushed however, let me elaborate.

I reviewed ks test implementation and I think it's correct. The idea is that you pass sample (extremes), distribution, and fit parameters. Fit parameters are used to convert distribution's CDF function of quantiles + fit parameters to a function of only quantiles - this produces a function of only quantiles. Fit parameters has all parameters (free and fixed). Because of this, we shouldn't be modifying the KolmogorovSmirnov class itself.

Regarding the original issue. Pyextremes EVA class works by doing the following:

1. Extract extremes
2. Transform extremes to standard form expected by distribution (e.g. minima to pseudo-maxima) - this is done with the ExtremesTransformer class
3. Fit distribution to transformed extremes - this is where terminology goes a bit haywire (my bad). Distribution has:
   • fit_parameters attribute, which has estimated parameters
   • fixed_parameters, which has frozen (e.g., floc) parameters.
4. Get extreme values by calculating ISF for given exceedance probability and then transforming the calculated value back (remember step 2) to original extreme value domain

Where the issue truly lies is how the EVA.test_ks method is implemented. To summarize:

• KolmogorovSmirnov expects a sample, distribution, and all (free and fixed) parameters
• EVA has raw extremes and transformed extremes (we must use transformed because that's what fit parameters are for)
• EVA.model has fit parameters which are only free parameters (bad choice of terminology on my end)

What we need to do is to change the EVA.test_ks method to pass the following to KolmogorovSmirnov:

• self.extremes_transformer.transformed_extremes to extremes
• self.model.fit_parameters (this is the same as self.model.distribution.mle_parameters) combined with self.model.distribution.fixed_parameters to fit_parameters - this way we will be passing all (free and fixed) parameters

My apologies for sending you on a wild goose chase earlier. Please let me know if you would be willing to revert the changes and update code.

[eastjames]

George, thank you for the clarification. I see why changes to KolmogorovSmirnov class aren't needed -- I've changed the PR to only have the necessary changes to EVA.test_ks.

Using the term fit_parameters to mean (very slightly) different things in different places of the code is a bit confusing. However, you've already noted this and I think it is outside the scope of this PR!