Closed nhpackard closed 5 years ago
@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?
@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...
Sorry I haven't been working on / reviewing the notebook stuff. I'll have a few hours Monday morning to see what's been done, and I'll try to get the GitHub collaboration set up as well.
On Sat, Apr 13, 2019 at 12:02 PM ggazzola notifications@github.com wrote:
@nhpackard https://github.com/nhpackard, also:
- Incorporated all of @Gianluca https://github.com/Gianluca's commits
- Incorporated most of @Gianluca https://github.com/Gianluca's suggestions in [ProtoLife/pdt-server#479 https://github.com/ProtoLife/pdt-server/issues/479]
That Gianluca you are hyperlinking to is some other GitHub user...
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ProtoLife/daptics-api/pull/3#issuecomment-482866722, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAdmH94WuHgMpwSWfEr3KlT1NjjC7Jaks5vgim1gaJpZM4crI3h .
-- Peter Zingg
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
is_demo
does and whether we should let the user change it.Step 4
.../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
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)
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, @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.
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.
[x] 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.- [x] 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 theHOME
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 executedjupyter 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 whatself.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 seedesign=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 !!)
@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.