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

Add base_sequence to sample.sobol #617

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tupui
Copy link
Member

@tupui tupui commented Apr 22, 2024

I had to do this for a project to change the sampling method.

I am not sure about this and maybe we should start thinking about using the new SciPy machinery.

What do you think?

@ConnectedSystems
Copy link
Member

ConnectedSystems commented Apr 25, 2024

I am not sure about this and maybe we should start thinking about using the new SciPy machinery.

Do you mean replace this implementation with what is in SciPy completely?

@tupui
Copy link
Member Author

tupui commented Apr 25, 2024

Yeah to see how the new functions can be used on our side. Same goes for Sobol' indices themselves

@ConnectedSystems
Copy link
Member

Sorry for the long delay in response.

I wonder if we can make the base_sequence also accept a function that, if given a target shape, will generate the array accordingly:

target_shape = (2 * D, N)
seq = base_sequence(target_shape)

Just a thought...

Otherwise I am fine with the current implementation

@tupui
Copy link
Member Author

tupui commented Sep 9, 2024

I see what you mean. I am not sure, do you have a use case yourself? This param is already questionable 😅 I don't mind adding though if you want. At some point we might either way to revisit the whole API (sad CZI 😓)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants