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

Issue tracker for Tutorial 01 #3934

Closed
jngrad opened this issue Oct 4, 2020 · 3 comments · Fixed by #3993
Closed

Issue tracker for Tutorial 01 #3934

jngrad opened this issue Oct 4, 2020 · 3 comments · Fixed by #3993
Assignees

Comments

@jngrad
Copy link
Member

jngrad commented Oct 4, 2020

List of things to fix based on feedback from the summer school.

  • matplotlib plots
    • should be instantiated with fig = plt.figure(figsize=(10, 8)) instead of fig = plt.figure() to be more readable by taking the entire width of the HTML div
    • should end with plt.show() to avoid printing the return value of the last matplotlib method
  • scipy
    • should be imported with import scipy.optimize in the cell where it is used for the first time, i.e. the def autocor() cell
  • autocorrelation function
    • the function gives a different result than what is calculated in the core with the Correlator accumulator, because the normalization doesn't consider that for larger tau values, the sample size is smaller and the variance higher
    • numpy and scipy are known to be tricky to use for scripting autocorrelation (de Buyl 2018, DOI:10.21105/joss.00877)
    • the long-term solution here would be to install tidynamics in the docker image
  • style
    • apply PEP8 formatting
    • assert don't need round brackets
@RudolfWeeber
Copy link
Contributor

@jgrad @schlaicha

@jngrad
Copy link
Member Author

jngrad commented Nov 2, 2020

Also, the tutorial implements the cyclic autocorrelation function:

import numpy as np
import tidynamics
import matplotlib.pyplot as plt

def autocor(x):
    x = np.asarray(x)
    mean = x.mean()
    var = np.var(x)
    xp = x-mean
    corr = np.correlate(xp, xp, 'full')[len(x)-1:]/var/len(x)
    return corr

x = np.arange(256)
y = np.sin(x * np.pi/16) / (x+1)**2

acf_td = tidynamics.acf(y) / np.var(y)
acf_tut = autocor(y) * len(x) / (len(x) - np.arange(len(x)))
print(np.sum(np.abs(acf_td - acf_tut)))

plt.plot(acf_td, label='tidynamics')
plt.plot(acf_tut, label='tutorial')
plt.legend()
plt.show()

@jngrad jngrad self-assigned this Nov 13, 2020
@jngrad
Copy link
Member Author

jngrad commented Nov 13, 2020

* `assert` don't need round brackets

Adding round brackets can lead to subtle bugs:

delete_precious_data = False
assert(delete_precious_data == True, "I can't delete data")  # tuple evaluates to true
cleanup_data()  # goodbye precious data

In python 3.6+ this construction prints SyntaxWarning: assertion is always true, perhaps remove parentheses? without halting the flow of the script. It also cannot be caught by a try/except without first importing module warning and writing some boilerplate code (see https://stackoverflow.com/a/15934081). In batch jobs submitted to a scheduler, the warning could be hidden deep in the logs.

@kodiakhq kodiakhq bot closed this as completed in #3993 Nov 16, 2020
kodiakhq bot added a commit that referenced this issue Nov 16, 2020
Description of changes:
- remove crystallization tutorial (partial fix for #3955, closes #3820)
- remove tutorial numbers and indicate difficulty level (fixes #3959)
- resolve issues in tutorial LJ (fixes #3934)
- create a tool to simplify editing exercise2 notebooks by temporarily converting solution markdown cells to executable code cells (fixes #3930) and apply pep8 formatting
  ```sh
  # convert solution cells to code cells
  ./pypresso doc/tutorials/convert.py exercise2 --to-py doc/tutorials/lennard_jones/lennard_jones.ipynb
  # edit the notebook
  ./ipypresso notebook
  # convert solution cells back to markdown cells
  ./pypresso doc/tutorials/convert.py exercise2 --to-md doc/tutorials/lennard_jones/lennard_jones.ipynb
  # apply autopep formatting rules
  ./pypresso doc/tutorials/convert.py exercise2 --pep8  doc/tutorials/lennard_jones/lennard_jones.ipynb
  ```
- implement an autocorrelation function `espressomd.analyze.autocorrelation()` to calculate the ACF of 1-dimensional and 2-dimensional data
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants