Skip to content

First attempt at incorporating sampling for uniformity (Morris)#247

Open
JoerivanEngelen wants to merge 2 commits intoSALib:mainfrom
JoerivanEngelen:sampling4uniformity
Open

First attempt at incorporating sampling for uniformity (Morris)#247
JoerivanEngelen wants to merge 2 commits intoSALib:mainfrom
JoerivanEngelen:sampling4uniformity

Conversation

@JoerivanEngelen
Copy link
Copy Markdown
Contributor

@JoerivanEngelen JoerivanEngelen commented May 23, 2019

For the original paper, see Khare et al. 2015
A multi-criteria trajectory-based parameter sampling strategy for the screening method of elementary effects

I thought I'll leave it as a pull request here, so we can discuss the code.

I had to hack the sampling strategy quite a bit to get this one to work. Goal was to use as much of the original SALib functions as possible. Currently the variable names I chose are quite bad and the code can be quite confusing. I also had to override some of the checks.
Most confusing is that the RepeatedSampling class inherits the Strategy class with:
num_pars=sampling4uniformity
and
k_choice=num_pars

For this to be more understandable, probably the original code has to be refactored a bit.

You can try it out yourself with for example:

param_values = sample(problem, N=16, grid_jump=2, num_levels=4,
                      optimal_trajectories=None, 
                      sample4uniformity = 300)

Here sample4uniformity sets what is "Q" in the paper, the number of repeated sampled trajectories.

To do:

  • e-mail original authors we are working on this
  • add better docstrings
  • better variable names
  • add checks, raise ValueErrors/Warnings
  • test whether this implementation actually provides a more uniform parameter spread
  • add tests
  • support groups
  • refactor?

@JoerivanEngelen
Copy link
Copy Markdown
Contributor Author

This should fix #131

@JoerivanEngelen
Copy link
Copy Markdown
Contributor Author

JoerivanEngelen commented May 24, 2019

I see that most of the tests now fail because no grid_jump is provided in the tests, which raises the question: why didn't they fail in previous commits?

…ormity and moved a warning so that it would not be called Q times.
@willu47
Copy link
Copy Markdown
Member

willu47 commented May 24, 2019

I see that most of the tests now fail because no grid_jump is provided in the tests, which raises the question: why didn't they fail in previous commits?

The grid jump argument was removed in an earlier release, as it is was computed directly from the num_levels parameter. This is why it doesn't appear in the tests.

@willu47 willu47 changed the base branch from master to main July 12, 2022 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants