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.

Sign up for GitHub

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

Jump to bottom

Bug in hard_sphere() ? #5

Closed
arbenede opened this issue Jul 5, 2018 · 2 comments
Closed

Bug in hard_sphere() ? #5

arbenede opened this issue Jul 5, 2018 · 2 comments
Assignees
@chrisvwx

Comments

@arbenede
Copy link

arbenede commented Jul 5, 2018

I am not sure if this is a bug or I am not understanding how to call hard_sphere() ...
If I run
x = hard_sphere([1 2]', [1 2; 3 4],2)
I get
x = [0; 1]
this solution gives the lattice point
[1 2; 3 4] * [0; 1] = [2; 4]
however there are 4 lattice points ([1 1], [0 2], [1 1], [1 1], [1 3], [2 2]) that are closer to [1 2] than [2 4].
Also, if I set the Nc input parameter to values >2 I get lattice points that move farther away from the closest point, so I am not sure what is going on...
@chrisvwx chrisvwx self-assigned this Jul 11, 2018
@chrisvwx
Copy link
Owner

Yes, you're right, there was a bug. Specifically, line 28 of hard_sphere.jl should be uncommented, and the following line removed. I'll fix this soon.

chrisvwx pushed a commit that referenced this issue Aug 11, 2018
Fix bug (see #5) in hard_sphere which was not returning the right ind…
9bb6807 
…ices.

Also updated for 1.0
chrisvwx pushed a commit that referenced this issue Aug 11, 2018
Fix bug (see #5) in hard_sphere which was not returning the right ind…
eaf6b9f 
…ices.

Also updated for 1.0
This was referenced Aug 26, 2018
Closed
Merged
chrisvwx added a commit that referenced this issue Aug 26, 2018
@chrisvwx
Updates for Julia 1.0
536ebb2 
* Fixes bug (see #5) in hard_sphere which was not returning the right indices.
* Added more explicit description of the arguments to hard_sphere
* Updated benchmarks, README to match recent changes. Added .toml files
@chrisvwx
Copy link
Owner

It wasn't especially soon, but this is fixed. I haven't yet updated the METADATA.jl repo so (assuming you're on 0.7 or 1.0), the easiest way to see the change is with the following command:
Pkg.add(PackageSpec(url="https://github.com/christianpeel/LLLplus.jl", rev="master"))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chrisvwx chrisvwx

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@arbenede @chrisvwx