Skip to content

Feature/form factors #92

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

Merged
merged 29 commits into from
Oct 12, 2020
Merged

Feature/form factors #92

merged 29 commits into from
Oct 12, 2020

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Oct 6, 2020

Description

This PR adds the ability to compute the form factor for a shape. The calculation is currently implemented for spheres, polygons, and polyhedra.

Motivation and Context

The code in ft.py is important to maintain in some form since it represents a relatively unique contribution and may be of future use for purposes of high fidelity diffraction calculations for anisotropic shapes. However, the code quality, style, and correctness could benefit from bringing into the fold of the newer object-base shape API.

Types of Changes

  • Documentation update
  • Bug fix
  • New feature
  • Breaking change1

1The change breaks (or has the potential to break) existing functionality.

Checklist:

@vyasr vyasr added enhancement New feature or request refactor labels Oct 6, 2020
@vyasr vyasr self-assigned this Oct 6, 2020
@vyasr vyasr requested review from DomFijan and bdice October 6, 2020 17:12
@vyasr
Copy link
Contributor Author

vyasr commented Oct 6, 2020

@DomFijan I'd be curious to get your thoughts on these calculations. The content of this PR is a means for evaluating the form factor amplitudes for a few shapes (spheres, polygons, and polyhedra). The sphere calculation is very straightforward, but the polyhedron calculation (and by extension, the polygon calculation it uses) were original contributions from one of our old grad students @eirrgang. The code has been shuffled around to various different parts of the glotzerlab code base as we've more clearly delineated the scope and goals for different packages over time. This package focuses on shapes, so it seemed like the best place to retain a reference version of this calculation.

Of course, it's not particularly practical; the implementation here is the amplitude for just a single shape, whereas a realistic form factor calculation would need to accumulate over many particles and then take the complex norm to get the observable intensity. Since that's out of scope for this package, I wanted to maintain this code here as a reference implementation in case we eventually want to implement this directly in something like glotzerlab/freud#660. Having said all that, your perspective on the utility of the code in this form and what might be useful to provide would be helpful.

@bdice you don't really need to review this, I'm just tagging you here so you're aware of it since this is relevant to your ongoing structure factor work and you originally copied this code into coxeter. Review if you'd like, but don't waste your time if you're busy.

@vyasr vyasr removed the request for review from bdice October 6, 2020 17:22
@vyasr vyasr mentioned this pull request Oct 6, 2020
Copy link
Member

@bdice bdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments but looks good overall! Thanks for porting this code over.

@bdice
Copy link
Member

bdice commented Oct 9, 2020

@vyasr vyasr merged commit af8b7c2 into master Oct 12, 2020
@vyasr vyasr deleted the feature/form_factors branch October 12, 2020 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants