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

Remove experiment from Trial hash #950

Merged
merged 13 commits into from
Jun 21, 2022

Conversation

bouthilx
Copy link
Member

It is problematic to include experiment in trial hash since the
experiment it belongs to is not known in advance in the algorithm
object. In any case, we can make a unique index combining trial.hash +
experiment.id so it is not necessary to hash the experiment.id.

The trial has now trial.id, the hash of params+parent+fidelity and
trial.id_override, the id corresponding to _id in legacy databases.
trial.legacy_id is added to keep a backward compatible hash with
previous versions of Oríon where trial.id included the experiment id in
the hash.

bouthilx added 4 commits June 20, 2022 14:52
It is problematic to include experiment in trial hash since the
experiment it belongs to is not known in advance in the algorithm
object. In any case, we can make a unique index combining trial.hash +
experiment.id so it is not necessary to hash the experiment.id.

The trial has now trial.id, the hash of params+parent+fidelity and
trial.id_override, the id corresponding to _id in legacy databases.
trial.legacy_id is added to keep a backward compatible hash with
previous versions of Oríon where trial.id included the experiment id in
the hash.
@bouthilx bouthilx added the enhancement Improves a feature or non-functional aspects (e.g., optimization, prettify, technical debt) label Jun 20, 2022
bouthilx added 9 commits June 20, 2022 15:20
Why:

When upgrading the DB to support index of (trial.id, trial.experiment),
the DB will end up with a mixed format for the trial ids. The previous
ids (_id) where the hash of the trials. It will now rely on the DB to
generate the next _id. Thus, it will have string index mixed with
integer values. It is best to keep ids as they were to better support
backward compatibility (and Oríon version down-grade), so we should
support mixed indexes. The bug was confusing anyway.

How:

Only consider integer indexes when inferring next id to use.
@bouthilx bouthilx merged commit 632a4f9 into Epistimio:develop Jun 21, 2022
@notoraptor notoraptor mentioned this pull request Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves a feature or non-functional aspects (e.g., optimization, prettify, technical debt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants