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

Notebook rewrite #3

Merged
merged 3 commits into from
Apr 17, 2019
Merged

Notebook rewrite #3

merged 3 commits into from
Apr 17, 2019

Conversation

nhpackard
Copy link
Contributor

@nhpackard nhpackard commented Apr 12, 2019

  • Incorporated all of @ggazzola 's commits
  • Incorporated most of @ggazzola's suggestions in ProtoLife/pdt-server#479.
  • Added structure for individual steps
    • connect/logon
    • ESD
    • Initial Exp
    • Loop
      • fill responses
      • save experiments make new design
  • Added final section for full loop in one cell.

@ggazzola
Copy link

@nhpackard, @pzingg: I now recall why I previously had to use forking, rather than the usual edit/commit/push, in daptics-api: I don’t have writing permissions for that repo. Could you please grant them to me?

@ggazzola
Copy link

@nhpackard, also:

  • Incorporated all of @Gianluca's commits
  • Incorporated most of @Gianluca's suggestions in [ProtoLife/pdt-server#479]

That Gianluca you are hyperlinking to is some other GitHub user...

@pzingg
Copy link
Collaborator

pzingg commented Apr 14, 2019 via email

@ggazzola
Copy link

Great rewrite! There are a few minor things I would change in the documentation language, but once you guys grant me writing privileges to this repository, I can make those changes myself in this PR, and then have you review them.

Also, below are some issues I found.

General

  • The tutorial should not charge credits. I'm guessing a good way to prevent this by not accepting ESD files that don't fit in the free zone.

  • I seem to understand the current API credit balance does not coincide with the web-interface one: how can I add credits to my API balance?

Step 3

  • I still don't get what is_demo does and whether we should let the user change it.

Step 4

  • "The current location of this tutorial is .../daptics-api/python_client/. In this case it should appear in the HOME tab of the jupyter notebook.": I don't understand what "in this case" means. Also, what does "it" refer to here?

Step 5

  • # or self.daptics.initial_params['populationSize']/ # or self.daptics.initial_params['replicates']: explain what self.daptics.initial_params is or, for simplicity, remove these two comments (I recommend the latter).

Step 7

The output of the daptics.print() command includes a design: I don't understand what that design corresponds to, since Step 7 is just an initialization step (and it preceeds the step that takes care of the initial experiments)

Step 8

  • daptics.random_experiments_with_responses fails if space is of mixture type (the function seems to be coded for factorial experiments only).

Step 9

  • This step is referred as "optional", but I seem to understand it is required if Step 8 was skipped, and must otherwise be skipped. Is that correct? If so, Step 9 should be referred to as "to be run only if Step 8 was run" or something.

  • "Note: if you call save_initial_experiments_csv(), you must have a file with at least the headers.": not clear what this means.

Step 10

  • "Subsequent calls to poll_for_current_task() will result in an error because no task will be found.": I tried calling poll_for_current_task() after a task was completed, and that didn't result in any error, but just "silence". Is that the expected behavior? If so, the above sentence should be accordingly updated, or maybe even removed.

Full loop

  • Can you add a little documentation about this loop? E.g., it replaces steps x–y, etc.

  • When you open a fresh notebook for the first time, the loop code is already followed by a bunch of printed lines ((status = success...), etc.): can you remove those lines from the notebook?

  • On a couple of occasions, the loop froze. In one case, the last printed lines were

Saving design to:  gen1_design.csv
status = running -- 27 seconds.

After waiting for a few minutes, I clicked the stop button on top of the notebook, and I got an error message that referred to a time-out error (unfortunately, I forgot to copy the message).

In another case, the last printed lines were:

Saving design to:  gen3_design.csv
status = running -- 29 seconds.Problem finding current task!
Error:  HTTPConnectionPool(host='inertia.protolife.com', port=8080): Read timed out. (read timeout=None)

@ggazzola
Copy link

By the way, I skimmed through but didn't quite review 04_GetAnalytics.ipynb or 05_RestartSession.ipynb: I'm guessing parts of those files will be eventually incorporated into 03_SimpleTutorial.ipynb, correct?

@nhpackard
Copy link
Contributor Author

@nhpackard, @pzingg: I now recall why I previously had to use forking, rather than the usual edit/commit/push, in daptics-api: I don’t have writing permissions for that repo. Could you please grant them to me?

Originally posted by @ggazzola in #3 (comment)

Oh man! Sorry about that. Should be fixed.

@nhpackard
Copy link
Contributor Author

nhpackard commented Apr 16, 2019

General

  • The tutorial should not charge credits. I'm guessing a good way to prevent this by not accepting ESD files that don't fit in the free zone.
  • Yes! I am going to change the tutorial ESD to fit in the free zone and add a comment.
  • I seem to understand the current API credit balance does not coincide with the web-interface one: how can I add credits to my API balance?
  • ultimately API credit balance will be hard-linked to the web-interface credit balance. At the moment this is not the case; basically a migration issue. Peter and I are discussing.

Step 3

  • I still don't get what is_demo does and whether we should let the user change it.
  • Peter says it should work just like in the web-interface (overwrite any user ESD with the demo ESD). I'm inclined drop it, because we have the free zone and it would be a pain to explain.

Step 4

  • "The current location of this tutorial is .../daptics-api/python_client/. In this case it should appear in the HOME tab of the jupyter notebook.": I don't understand what "in this case" means. Also, what does "it" refer to here?
  • The way jupyter notebooks work is that you start a server running (jupyter notebook at the command line) in some directory, typically where your notebooks are. In your case, you probably navigated to .../daptics-api/python_client/ and executed jupyter notebook. Then in the HOME tab of the notebook as it appears in your browser (when you go to http://localhost:8888 or wherever your notebook server tells you to go), you see the contents of the directory you started the server in, including the notebooks (at the top of the contents listing). "It" refers to the tutorial notebook, which should be one of those shown in the HOME tab. I'm not sure I should really go into all this detail on how jupyter notebooks work; maybe in the more introductory 01 or 02 notebooks.

Step 5

  • # or self.daptics.initial_params['populationSize']/ # or self.daptics.initial_params['replicates']: explain what self.daptics.initial_params is or, for simplicity, remove these two comments (I recommend the latter).
  • OK

Step 7

The output of the daptics.print() command includes a design: I don't understand what that design corresponds to, since Step 7 is just an initialization step (and it preceeds the step that takes care of the initial experiments)

  • you can always call daptics.print(). If you do so before you have created a design, then you should see design=None.

Step 8

  • daptics.random_experiments_with_responses fails if space is of mixture type (the function seems to be coded for factorial experiments only).
  • Bug, maybe fixed by one of Peter's recent commits that haven't made it to this branch yet.

Step 9

  • This step is referred as "optional", but I seem to understand it is required if Step 8 was skipped, and must otherwise be skipped. Is that correct? If so, Step 9 should be referred to as "to be run only if Step 8 was run" or something.
  • OK
  • "Note: if you call save_initial_experiments_csv(), you must have a file with at least the headers.": not clear what this means.
  • I'm not sure either. I took it out.

Step 10

  • "Subsequent calls to poll_for_current_task() will result in an error because no task will be found.": I tried calling poll_for_current_task() after a task was completed, and that didn't result in any error, but just "silence". Is that the expected behavior? If so, the above sentence should be accordingly updated, or maybe even removed.
  • There should be an error message. Now fixed.

Full loop

  • Can you add a little documentation about this loop? E.g., it replaces steps x–y, etc.
  • ok
  • When you open a fresh notebook for the first time, the loop code is already followed by a bunch of printed lines ((status = success...), etc.): can you remove those lines from the notebook?
  • output should not be there... will be gone.
  • On a couple of occasions, the loop froze. In one case, the last printed lines were
Saving design to:  gen1_design.csv
status = running -- 27 seconds.

After waiting for a few minutes, I clicked the stop button on top of the notebook, and I got an error message that referred to a time-out error (unfortunately, I forgot to copy the message).

In another case, the last printed lines were:

Saving design to:  gen3_design.csv
status = running -- 29 seconds.Problem finding current task!
Error:  HTTPConnectionPool(host='inertia.protolife.com', port=8080): Read timed out. (read timeout=None)
  • bug and/or timeout param adjustment needed (help @pzingg !!)

@ggazzola
Copy link

@nhpackard: thanks for the writing privileges. Please review my changes (several in the documentation, plus a few function calls).

A couple more comments:

  • How about replacing the 4 x 8 ESD with a 4 x 4 or so? With the 4 x 8, each generation takes 40+ seconds to process...

  • Step 6. When you execute the first cell (while True: ...), you now get:

status =  success
No current task found!

When you execute then the second cell (daptics.wait_for_current_task()), you now get:

Error: no client found!

The latter happens also with step 10.

nhpackard and others added 2 commits April 16, 2019 16:51
revising notebook...

revising notebook again

rewrite of 03_SimpleTutorial.ipynb + changes in daptics_client.py

adding some error reporting

changed loop to use esd-factorial-8x5 to avoid space size error

add documentation string for wait_for_current_task()

Corrections for latest GG feedback

Eliminate output, enhance comments

Edits to documentation.

More edits to documentation.

More documentation edits, typo fixes, etc.

More documentation about free zone and loop automation.

Commented out mixture example, edited loop documentation, set all execution_count to null.

Added back login_data.

Loop with free ESD.

demo parameter now hardcoded in create_session.

is_demo removed from function call within loop.

Capitalization.

Capitalization.
Copy link
Collaborator

@pzingg pzingg left a comment

Choose a reason for hiding this comment

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

This is now ready to merge.

@nhpackard nhpackard merged commit 03e1cfe into master Apr 17, 2019
@pzingg pzingg deleted the notebook-dev branch May 16, 2019 01:21
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

3 participants