Major rework of KLTrialFactory and internal trial handling#37
Merged
Major rework of KLTrialFactory and internal trial handling#37
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Description
This PR is a major overhaul of how klibs handles trials and trial generation internally, making it much easier to generate custom blocks of trials and improving the public API while largely preserving backwards compatibility. These changes also do most of the groundwork needed to add an official custom block/session structure API.
On an internal level, a big change is that the
BlockIteratorandTrialIteratorclasses are now gone and have been replaced with the newTrialSetclass, which is fully documented and intended for public use. Previously, theTrialIteratorclass was responsible for recycling trials during the experiment runtime, which was a bit confusing and messy. The trial recycling code has now been rewritten and moved into theExperimentclass itself, making things much simpler and allowing theTrialSetclass (a drop-in replacement forTrialIterator) to simply be a container.TrialSetalso allows for the specification of block labels, which are used to fillself.block_labelattribute during a block. This is intended to make it easier to write conditional logic for tasks with different phases or types of blocks.This PR also does a lot of work to simplify the
TrialFactoryclass and remove it from the public API, making its most common uses available through theExperimentclass. This includes moving the loading of experiment factors intoExperiment.__init__()and making the factors accessible directly throughself.exp_factorswithin the runtime (previously requiredself.trial_factory.exp_factors). Additionally, theTrialFactory.dump()method has been deprecated and replaced byExperiment.generate_trials_txt(), which has also been heavily rewritten for better readability and usefulness. The class itself and its basic design are still useful internally (I tried moving more of it to theExperimentclass but it wasn't as clean), it's just something I don't think end users should ever have to think about.Also, this PR adds a bunch of unit test coverage for generating trials within the experiment runtime.
Merge Checklist
closes #<issue-number>to automatically close an issue