Skip to content

Conversation

Cengoni
Copy link
Contributor

@Cengoni Cengoni commented Dec 2, 2020

No description provided.

@codecov-io
Copy link

codecov-io commented Dec 2, 2020

Codecov Report

Merging #11 (088fac4) into master (ed252e7) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #11   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         3    +1     
  Lines            4        65   +61     
=========================================
+ Hits             4        65   +61     
Impacted Files Coverage Δ
src/STMOZOO.jl 100.00% <ø> (ø)
src/cuckoo.jl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed252e7...088fac4. Read the comment docs.

@MichielStock
Copy link
Owner

Very tidy code and well done! I can only give very nitpicky suggestions:

  • consider using Unicode symbols for Greek letters such as alpha, gamma, etc (this is a question of style, you might prefer it like it is).
  • Maybe you could also give an example of more than 2 dimensions in the notebook? Or a slightly more hard function?
    Otherwise, great job. My suggestions are in a PR.

@Cengoni
Copy link
Contributor Author

Cengoni commented Dec 24, 2020

Thank you for your suggestions, I will work on them! And to answer your question about why I used Distributions to pick a random value from a normal distribution is because in base randn it seems I can't specify the variance

@MichielStock
Copy link
Owner

sigma * randn() + mu ;-)

@MichielStock
Copy link
Owner

Needs to be reviewed tomorrow by @francops1722 and @bspanoghe

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.

4 participants