Closed brookehus closed 6 years ago
@brookehus Thanks very much!
In the last code block, I got: /Users/brooke/miniconda3/envs/pyemma/lib/python3.6/site-packages/pyemma/plots/plots2d.py:248: UserWarning: zorder=-1 is not an allowed optional parameter and will be ignored ' be ignored'.format(key, kwargs[key]))
That will be fixed with the next PyEMMA release on conda.
[08] This code block throws an error which is expected:
I'd imagine that it could be more difficult to understand if we put it into a try/except context... If people read the text, it should not be a problem. Closing this.
[06] For hmm_6 I get: UserWarning: confidence interval for constant data is not meaningful warnings.warn('confidence interval for constant data is not meaningful')
Linking this to #37
Also, the plot in the last code block looks too narrow to me in the vertical direction
This, unfortunately, requires a nontrivial change in PyEMMA.
If I'm not mistaken, we've incorporated or at least discussed why we didn't incorporate the comments here. Citations are covered by #105 . Closing this, feel free to re-open.
This is all pretty high level! I do not mind at all if you ignore all/most/any of these suggestions. I didn't have any widgets because I just cloned the notebooks from source. FYI, I haven't looked at any updated version of
00
. I also apologize if my versions are 24h out of date and therefore some of my suggestions are irrelevant.01 and in general
#FIXME
, but it may be helpful for the solutions to just have a comment at the top saying#Worked solution
or something.instead of just printing it, because then each entry will be its own row and easier to read. For example with describing features, or printing trajectory files. Or use pprint.pprint for this purpose (which is the default, if the last element of a cell is formatted by jupyter - @marscher)
02
This time shift has a default value of 10 steps, but it is always a good idea to specify it explicitly.
", but then I seelag=1
specified in the relevant command. Is this correct?logscale=True
. Same for the density plot in the exercises with ala2 + PCA as well as ala2 + TICA. I don't feel as strongly about using a logscale for the density in the pentapeptide dataset, but maybe for consistency it's better. Definitely for the lag time plots the logscale is helpful for the density ones.03
Some cells take longer than an instant to run. Maybe it's worth a comment sometimes if a cell is going to take especially long?[Could not reproduce here, changed in other NBs]04
05
Also, the plot in the last code block looks too narrow to me in the vertical direction06
timescales_hmsm
plot that compares the poor and good discretization? I see that later in the notebook it's explained that HMMs are robust to errors in discretization. But maybe good to foreshadow this so I'm not reading too much into the plots.For[will be fixed in PyEMMA, opened a github issue, cf. below]hmm_6
I get:07
08
.active_set
. In this example we clearly see that some states are missing.". It would be nice to print out the percent active too!I wonder if a try/except formulation would be better, with a helpful printout, so the user isn't disoriented by the error.