Update unsequa to fully support the latest version of SALib

[luseverin]

Changes proposed in this PR:

• Update SALib sensitivity and sampling methods from newest version (SALib 1.4.7) #828
• Allow for computation of relative and absolute delta impacts in CalcDeltaClimate

This PR fixes #828

PR Author Checklist

• [x] Read the Contribution Guide
• [x] Correct target branch selected (if unsure, select develop)
• [x] Descriptive pull request title added
• [x] Source branch up-to-date with target branch
• [ ] Documentation updated
• [x] Tests updated
• [x] Tests passing
• [x] No new linter issues
• [x] Changelog updated

PR Reviewer Checklist

• [ ] Read the Contribution Guide
• [ ] CLIMADA Reviewer Checklist passed
• [ ] Tests passing
• [ ] No new linter issues

[luseverin]

A few more comments on this PR:

• Three new sensitivity methods required a specific treatment to be included into unsequa: ff, hdmr, and delta
• The output of the ff and hdmr methods from SALib required refactoring in order to fit unsequa's _sens_df output format. I had to remove a few variables from the output of hdmr as I could not find a way to refactor them to the _sens_df. These include notably all the details of the surrogate model used, included in the Em (Emulator) variable. Additionally, all output variables from hdmr are refactored to 2D arrays, even the first-order sensitivity indices.
• The delta method systematically failed with a matrix inversion error: LinAlgError: Singular matrix detected This may be due to the sample size (30000) being too small. If this is not the case, check Y values or raise an issue with the SALib team. I could not find why that was so I removed delta from the compatible sensitivity methods.
• I also have some doubts on the stability of the hdmr method as it seem to yield different results between identical model runs. I decreased the sensitivity of the AssertEqual tests in the unit test in consequence so that it should not be failing.

[luseverin]

Great, thanks!

I think there are a few small things to update, and maybe one general idea for the tests. Since the tests for the different method repeat themselves, it would be better to define a function and then call it in a loop for all methods instead of copying large parts of the code.

Ok so I included your suggested changes + I added a prototype of a generic testing function for testing the different sensitivity methods. I store all the parameters and expected tests results in a dict which I pass to the function that gets looped over. Maybe not the most efficient way to do it so please let me know if you have any suggestions for improvements.

[chahank]

Great job, we are almost there! What is missing:

• Address all the new linter issues (and any older ones that you solve are bonus points!)
• The method for the test is in the right direction. I would maybe propose to simplify the whole thing a bit. More on that in the code directly.

[luseverin]

Ok so I tried to simplify the tests of the sensitivity methods as much as possible in the generic test functions, and removed the previous method-specific test functions. I wasn't sure about a few lines that only appeared in the test for the morris method though:

for name, attr in unc_data.__dict__.items():
            if 'sens_df' in name:
                np.testing.assert_array_equal(
                    attr.param.unique(),
                    np.array(['x_exp', 'x_paa', 'x_mdd'])
                    )
                np.testing.assert_array_equal(
                    attr.si.unique(),
                    np.array(['mu', 'mu_star', 'sigma', 'mu_star_conf'])
                    )
                if 'eai' in name:
                    self.assertEqual(
                        attr.size,
                        len(unc_data.param_labels)*4*(len(exp_unc.evaluate().gdf) + 3)
                        )
                elif 'at_event' in name:
                    self.assertEqual(
                        attr.size,
                        len(unc_data.param_labels) * 4 * (haz.size + 3)
                        )
                else:
                    self.assertEqual(len(attr),
                                     len(unc_data.param_labels) * 4
                                     )

I did not include these lines in the generic test function as it did not seem absolutely necessary to me, but if you think those should be included just let me know.

Additionally, I need to fix the few new linter issues that popped up but I think it shouldn't be an issue. Otherwise I think I adressed the rest of your comments.

[luseverin]

Ok so I incorporated your last suggestions. If there is no further comments from your side I suggest we merge this PR?

[chahank]

I think it looks good!

Just FYI: you managed to remove 39 pylint warnings, and add 16. Maybe you can take a last look whether the 16 new ones are easy to avoid or not.

Else, this is ready to merge from my point of view.

[luseverin]

So I have checked and the warnings are mainly "too-many-locals", "too-complex", "invalid-name", etc. Most of those are coming from lines that I not directly touched or that got included to the commits because of trailing whitespaces that got removed by VScode. For instance, plot_rp_uncertainty is too complex, or they are too many local variables in Calc_delta_climate.uncertainty. It is not straightforward to me how these should be treated so I would rather leave them as is for the moment if that's fine..

[chahank]

Thanks for checking. For the moment these are fine I think.

Feel free to merge, and thanks for the work!