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

Make change how grid indexing tuples are saved so that find_gridpoints() finds anything #22

Merged
merged 2 commits into from
Mar 21, 2024

Conversation

Jacob-Stevens-Haas
Copy link
Owner

@Jacob-Stevens-Haas Jacob-Stevens-Haas commented Mar 20, 2024

See #21. I'm going to leave that open after this PR to address some of the thornier questions this issue raises: the opportunity for spurious matches. This PR also fixes an unrelated bug in find_gridpoints()

TL;DR: @yb6599 the experiments you were running should work now.

There has been some ambiguity about what index is stored in gridsearch results.
Previously, GridsearchResult contained the indexes of the optimal result in the
full series gridsearch array, which included an index of the metric as well
as the indexes to uniquely identify parameters at the gridpoint.  However,
the SavedGridPoint objects obviously only knew their parameter indexes, rather
than all carrying around indexes for every metric (which would be identical
across every point).

Now, early in gridsearch, the metric index is lost, meaning that every
GridsearchResult will store indexing tuples for the optimal results that
align with parameter indexes.

Also, rename np_to_primitive()
@yb6599
Copy link
Collaborator

yb6599 commented Mar 21, 2024

LGTM. I'll start running the experiments.

@Jacob-Stevens-Haas Jacob-Stevens-Haas merged commit 971d6cf into main Mar 21, 2024
5 checks passed
@Jacob-Stevens-Haas Jacob-Stevens-Haas deleted the hotfix-21 branch April 3, 2024 13:40
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.

2 participants