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

adding support for EXPFACTORY_EXPERIMENT_FIRST and EXPFACTORY_EXPERIMENT_LAST #162

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vsoch
Copy link
Member

@vsoch vsoch commented Mar 22, 2022

This should allow a container to specify EXPFACTORY_EXPERIMENT_FIRST and/or EXPFACTORY_EXPERIMENT_LAST/ It won't fail if it's messed up, but it will issue a warning! I haven't tested this because it's 6:20 am (am doing before work to get in something sooner than later) but I'll be able to tweak the PR / do more testing and updating docs after work in the evening.

Signed-off-by: vsoch vsochat@stanford.edu

Description of the Pull Request (PR):

Write your description of the PR here. Be sure to include as much background,
and details necessary for the reviewers to understand exactly what this is
fixing or enhancing.

This fixes or addresses the following GitHub issues:

  • Ref: #

Checkoff for all PRs:

Attn: @expfactory-admin

…ENT_LAST

Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch vsoch mentioned this pull request Mar 22, 2022
@danieljwilson
Copy link

So pulling this branch is working but I was wondering if I am also specifying EXPFACTORY_EXPERIMENT_FIRST (or LAST) in the Dockerfile as well? And if so does it matter where I put it?

@vsoch
Copy link
Member Author

vsoch commented Mar 22, 2022

So pulling this branch is working but I was wondering if I am also specifying EXPFACTORY_EXPERIMENT_FIRST (or LAST) in the Dockerfile as well? And if so does it matter where I put it?

yep you'd want to define it in your container, and it doesn't matter because it will be read when you start running the container (way after being built). Personally I'd put it with the other expactory variables just for organization.

@danieljwilson
Copy link

Sorry, I feel guilty to keep asking what I am sure are basic questions, but when you say to put them with the other expfactory variables are these the lines written after Experiments?

Like, for example:

########################################
# Experiments
########################################


LABEL EXPERIMENT_plus-minus /scif/apps/plus-minus
WORKDIR /scif/apps
RUN expfactory install https://github.com/expfactory-experiments/plus-minus

EXPFACTORY_EXPERIMENT_FIRST=sona-id-entry

ENTRYPOINT ["/bin/bash", "/startscript.sh"]
EXPOSE 5000
EXPOSE 443
EXPOSE 80

@danieljwilson
Copy link

I also noticed that there is an export command in the startscript.sh...

This is NOT where I would put it though as I understand it...

if [ "${EXPFACTORY_START}" == "yes" ]; then

    echo "Database set as ${EXPFACTORY_DATABASE}"
    export EXPFACTORY_DATABASE

@vsoch
Copy link
Member Author

vsoch commented Mar 22, 2022

Oh sorry! So Dockerfile you can define environment variables, and that would look like this:

ENV EXPFACTORY_EXPERIMENT_FIRST=sona-id-entry

I thought there were some others in there (toward the top?) you could use as examples, apologies for not being more clear!

@vsoch
Copy link
Member Author

vsoch commented Mar 22, 2022

here is the group I had in mind:

ENV EXPFACTORY_STUDY_ID {{studyid}}
ENV EXPFACTORY_SERVER localhost
ENV EXPFACTORY_CONTAINER true
ENV EXPFACTORY_DATA /scif/data
ENV EXPFACTORY_DATABASE {{database}}
ENV EXPFACTORY_HEADLESS {{headless}}
ENV EXPFACTORY_BASE /scif/apps

@danieljwilson
Copy link

Ah, got it! Thanks.

So I added that and it seemed to work for serving the desired first survey.

However with some additional testing I noticed a couple of things:

  1. If I build a new container (e.g. I did this to add a LAST in addition to the FIRST) it still serves the FIRST experiment first even if there is a ...results.json file in that subject's/token's folder on the server. Not sure if this is expected?
  2. If I designate a LAST experiment it seems to serve it immediately after the first - skipping all experiments in between...

@vsoch
Copy link
Member Author

vsoch commented Mar 23, 2022

If you want to look at the logic here https://github.com/expfactory/expfactory/pull/162/files#diff-1412625cab09f31db173d550c407c45dfd897747f259d2fe5126f9a6afe58b0dR134-R153 and make a suggestion, we could try that out. If a subject finishes an experiment and it's removed from the session variable, it shouldn't show up again (so that needs a debug). You could try unsetting it after it's selected, e.g.,:

            # Grab the first experiment if haven't done it yet
            if self.first_experiment and self.first_experiment in experiments:
                next = self.first_experiment
                self.first_experiment = None
            # Quickly grab the last experiment
            elif self.last_experiment and self.last_experiment in experiments:
                next = self.last_experiment
                self.last_experiment = None

            # If we have a last experiment, don't choose if there are > 1 left
            elif self.last_experiment and len(experiments) > 1:
                subset = [x for x in experiments if x != self.last_experiment]
                if self.randomize is True:
                    next = self.choose_random_experiment(subset)
                else:
                    next = subset[0]

            # Last cases, randomize or not!
            elif self.randomize is True:
                next = self.choose_random_experiment(experiments)
            else:
                next = experiments[0]

But if it's not in experiments (from the session) I don't see why that would be necessary.

This block in the above:

            # If we have a last experiment, don't choose if there are > 1 left
            elif self.last_experiment and len(experiments) > 1:
                subset = [x for x in experiments if x != self.last_experiment]
                if self.randomize is True:
                    next = self.choose_random_experiment(subset)
                else:
                    next = subset[0]

should be handling the case of having a last experiment and selecting from all of the other ones (e.g., subset doesn't include it) so you could add some prints there too to debug - let me know what you find!

@vsoch
Copy link
Member Author

vsoch commented Mar 23, 2022

I think the first bit might be a bug unrelated to the first/last addition - e.g., if you restart a server it's going to reset the list of experiments, and i doesn't take into account having been completed (or not) it's going to use the same session from your browser. This is usually okay for a participant because they will just run through things once, but could result in the behavior you see. You could try a different browser to test this, and if that's the case we'd want one more level of filter after get_experiments to not provide one that is finished.

@vsoch
Copy link
Member Author

vsoch commented Sep 26, 2022

ping @danieljwilson if you want further work or discussion on this?

@danieljwilson
Copy link

Thanks @vsoch - I think that this can be closed. Setting the first experiment seems to work and although I didn't resolve setting the last experiment it has not been critical for the projects I am doing or envision doing in the future. If that changes perhaps I will revisit at that time. Thanks for your help!

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.

None yet

2 participants