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

Reusable practice phase #92

Closed
BeritJanssen opened this issue Apr 19, 2022 · 5 comments · Fixed by #1360
Closed

Reusable practice phase #92

BeritJanssen opened this issue Apr 19, 2022 · 5 comments · Fixed by #1360
Assignees

Comments

@BeritJanssen
Copy link
Collaborator

BeritJanssen commented Apr 19, 2022

In the rhythm experiments, some logic for letting the participant practice was introduced, but this may not generalize well for other kinds of experiments. Ideally, we should have some utility functions for a practice phase which can be reused for any experiment which needs practice rounds. This practice phase defines the number of rounds the participants gets to practice each time, and a pass condition (i.e., how many correct results are required) before the participant enters the experiment proper.

@BeritJanssen BeritJanssen added this to the Nings categorization experiment milestone Apr 19, 2022
@BeritJanssen BeritJanssen changed the title Reusable practice view Reusable practice phase Apr 25, 2022
@Evert-R
Copy link
Contributor

Evert-R commented Sep 20, 2022

I have some ideas about this:

  • an extra field (string) in the session 'current_phase'
  • This field will be copied in the comment field of each result (or if this field is already used in other experiments an new 'phase' field)
  • A method in the session to collect the results per phase.
  • A method in the session to determine round number based on the current phase

@Evert-R Evert-R modified the milestones: Nings categorization experiment, Infrastructure Sep 20, 2022
@BeritJanssen
Copy link
Collaborator Author

BeritJanssen commented Sep 28, 2022

I personally think that the session.current_phase field might only make sense for a small portion of experiments, and will not add any information to the database that is interesting for users later -- they'll want to know which result is associated with which phase, and on session this field will be whatever value was set last. So I'm all for putting a phase field which is null=True, blank=True on the Result model. But if the comment field can be used for this for now, we might also just reuse this field, and perhaps give it a more descriptive name, e.g., "type"?

As far as I can see, all the reasoning will always go over the session.result_set anyway, or is there a direct advantage to knowing the session.current_phase which cannot be covered with the json_data? I know working with the json_data is unwieldy, but as far as I noticed, it's no bottle-neck to performance.

Again, if it's too inconvenient to work with existing fields (and I may be overlooking some problems!), go for it and add fields, but otherwise, I'd rather keep the models as lean and multi-purpose as possible.

@Evert-R
Copy link
Contributor

Evert-R commented Sep 28, 2022

I think you are right that we should only change this if it will make sense for a substantial number of future experiments. Working with the json_data as we do now does feel a bit counter intuitive, but indeed this doesn't seem to affect the performance, and does provide a lot of freedom to build whatever we want.

The comment field in the result is also fine as it is now, if we are not going to fill this with the current phase.

Maybe we can only make the comment field a bit easier to use then by adding it as an (optional) argument to prepare_result?
And also show the comment in the result in the django admin, cause I noticed it's not there.

@Evert-R
Copy link
Contributor

Evert-R commented Oct 6, 2022

I've made a subtask for the optional comment parameter in prepare results.

#248

@BeritJanssen BeritJanssen added the wontfix This will not be worked on label Feb 28, 2023
@BeritJanssen BeritJanssen reopened this Sep 30, 2024
@BeritJanssen BeritJanssen removed wontfix This will not be worked on 8h labels Sep 30, 2024
@BeritJanssen
Copy link
Collaborator Author

BeritJanssen commented Sep 30, 2024

I decided to still work on this issue. The rhythm experiments I originally implemented include some practice phase which has a lot of shared syntax, but the way it's implemented is pretty complex and depends on functions with a plethora of arguments: I'm afraid no-one will really know that the helper functions I implemented back then exist, or how to use them. I'm planning to move this to a dedicated practice.py rules file, from which experiments can inherit & perhaps piggy-back on some logic that's already been implemented.

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

Successfully merging a pull request may close this issue.

3 participants