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

feat: set up initial experiment server and cycle on firebase #342

Conversation

younesStrittmatter
Copy link
Collaborator

Description

Add experiment runner to the package

Type of change

  • feat: A new feature

Features (Optional)

  • add features to communicate with a firestore database to send conditions and retrieve dependent variables, added feature to increase the number of participants for a experiment on prolific to collect data

Questions (Optional)

  • how do we want to store experiment/prolific specific information, that might be sensible (tokens to talk to prolific and credentials to talk to firebase). The solution here just strores it in json files (maybe we can make one single json file for database name, name on prolific, tokens, credentials ... or maybe there are other/better solutions).
  • This is a first simple solution were the "get_data_function" would just wait until there is data in the database and then start the theorist, but we might want to add an additional "flag database" to make this more elaborate and to catch some errors.
  • There is a new dependency, "firebase_admin" that is needed to communicate to firebase, how do we handle this?
  • I am excited to run a first closed loop with this. The experiment is set up on Firebase already, all we would have to do is integrate this in the cycle and set up the experiment on prolific. Then we could already start collecting the first experiment.

@kphan20
Copy link
Contributor

kphan20 commented Apr 6, 2023

how do we want to store experiment/prolific specific information, that might be sensible (tokens to talk to prolific and credentials to talk to firebase). The solution here just strores it in json files (maybe we can make one single json file for database name, name on prolific, tokens, credentials ... or maybe there are other/better solutions).

I'm not sure if we already do this in the project, but we could save credentials and such with dotenv. It would allow us to define the credentials as environment variables.

@hollandjg
Copy link
Member

There is a new dependency, "firebase_admin" that is needed to communicate to firebase, how do we handle this?

You can add it to the pyproject.toml and poetry.lock files by calling poetry add firebase_admin in the root of the project.

@hollandjg
Copy link
Member

To use this together with the Controller, you'll need a single function which creates the experiment and returns the results as an array. I'm not sure I can see that yet (but let me know if I've not seen it).

@hollandjg
Copy link
Member

How do we want to store experiment/prolific specific information, that might be sensible (tokens to talk to prolific and credentials to talk to firebase). The solution here just strores it in json files (maybe we can make one single json file for database name, name on prolific, tokens, credentials ... or maybe there are other/better solutions).

For now, I'd just add them as parameters to the functions, which you can pass using a dictionary using **params.
I think the thing which calls the functions should deal with loading the parameters – the functions themselves should be completely agnostic about that.
The calling thing might be the Controller object from #305 (which handles passing parameters to different parts of the cycle), or a command line tool which initiates the runner, or a script which the human-in-the-loop writes.
That calling thing can load the parameters from a JSON/YAML/TOML file, or have them as part of the script, or from a dotenv as Kevin has suggested.

@hollandjg hollandjg changed the title 236 feat set up initial experiment server and cycle on firebase feat: set up initial experiment server and cycle on firebase Apr 7, 2023
Copy link
Collaborator

@musslick musslick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Only a couple of small comments.
One major item is naming: We should use the names for IVs and DVs as determined in the maintenance meeting.

Args:
collection_name: the name of the study as given in firebase
condition: the condition
firebase_credentials: dict with the credentials for firebase
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently this is typed as string but listed as dict argument in the comment?

doc_ref.set({"condition": condition_json})


def get_dependent_variable(collection_name: str, firebase_credentials: dict) -> Any:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just flagging for discussion: Depending on how we decide about the terminology (independent variables vs. conditions; observations vs. dependent variables), we should rename it. Currently, it's a mix of both (conditions, dependent_variabes).

condition: the condition
**kwargs:
"""
send_condition(kwargs["collection_name"], condition, kwargs["firebase_credentials"])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it sending a single condition or an entire set of conditions? If it is the former then we should make this more clear by labeling it accordingly (e.g., send_experiment or send_conditions.

**kwargs:

Returns:
the dependent variavle
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
the dependent variavle
the dependent variable

kwargs["collection_name"], kwargs["firebase_credentials"]
)
while data is None:
sleep(10)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the sleep time be a parameter?

@hollandjg
Copy link
Member

this is outdated, replaced by the firebase package

@hollandjg hollandjg closed this Jun 2, 2023
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.

feat: set up initial experiment server and cycle on firebase
4 participants