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

Modes not working properly in BondOrder #1201

Open
Charlottez112 opened this issue Jan 24, 2024 · 3 comments
Open

Modes not working properly in BondOrder #1201

Charlottez112 opened this issue Jan 24, 2024 · 3 comments

Comments

@Charlottez112
Copy link
Contributor

Charlottez112 commented Jan 24, 2024

Description

When I used modeslbod and oocd, I got the same result as mode bod, and got a 0 array when I used obcd. @Tobias-Dwyer also reported the same problem.

Steps to Reproduce

n_bins_theta = 100
n_bins_phi = 100
bod = freud.environment.BondOrder((n_bins_theta, n_bins_phi), mode='bod')

bod_array = bod.compute(
    system=traj[-1], neighbors={"num_neighbors": 12}
).bond_order

bod_array = np.clip(bod_array, 0, np.percentile(bod_array, 99))
plt.imshow(bod_array.T)
plt.show()

Error Output

Expect to see different BOD patterns when different modes are used.

freud Version

v2.13.2

Python Version

v3.10.13

System Platform

Linux

Installation method

Download from conda-forge

Developer

Would someone else please fix this?

@Charlottez112 Charlottez112 added the bug Something isn't working label Jan 24, 2024
@tommy-waltmann
Copy link
Collaborator

I am not personally familiar with the details of the different methods for BondOrder, and through offline discussions, it appears this bug has existed in freud since before the freud 2.0 release. @bdice @vyasr do you know who originally developed the different methods?

In any case, it would be helpful to attach the input data that triggers the faulty behavior so we can have a more in-depth look at this.

@Charlottez112
Copy link
Contributor Author

Charlottez112 commented Jan 25, 2024

@Tobias-Dwyer and I figured out that we didn't pass the orientations properly, so this is not a bug. But might be good to add a check (check if orientations are none when modes other than bod is used. Only mode bod doesn't need orientations) as for positions we can just pass a frame to system without having to pass positions and box separately?

If we decide to add this check I can work on it myself

@Charlottez112 Charlottez112 removed the bug Something isn't working label Jan 25, 2024
@tommy-waltmann
Copy link
Collaborator

I think the proper course of action here is to issue a warning when users don't explicitly give orientations if the mode is non-default.

Like most other compute methods in freud, the compute method in BondOrder takes a system object so you can pass either a tuple with (box, points) or a gsd frame/snapshot. This is described in the documentation here.

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

No branches or pull requests

2 participants