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

ADPs in structure factor calculation #34

Open
wants to merge 17 commits into
base: diffuse
Choose a base branch
from

Conversation

apeck12
Copy link
Collaborator

@apeck12 apeck12 commented Dec 1, 2017

Atomic displacement parameters implemented as an optional input to scatter.simulate_atomic, with the cpp_scatter.* files updated accordingly. Nose tests were added to test_scatter.py to validate
(1) the cases of no B factors, isotropic B factors, and anisotropic B factors,
(2) the CPU and GPU code, and
(3) the python interface
against a reference implementation. The code passes each of these tests, though please let me know if there are others I should add. Example output for the pentagon test model is below.

To benchmark this, I tried to modify and run the code at the end of cpp_scatter.cpp (specifically, updating the commented-out section under "// This is a meaningless test, for speed only..." to accommodate a U matrix), but it seemed not to work. Here's what I tried:
$ g++ cpp_scatter.cpp -o file1
$ ./file1
CPP OUTPUT:0.000000
0.000000
I'm not sure if that's the correct way to run the benchmark code, or if there's a separate issue.

test_bfactors

@tjlane
Copy link
Owner

tjlane commented Dec 3, 2017

Nice work!!! Took a look at the code and no comments there. I am pushing a commit that I hope will fix travis' complaining.

You did the "right" thing for the benchmark... I wasn't lying when I said it was meaningless. Typically I have compiled with the following (from compile_debug_programs.sh in the repo):

g++ -O3 -lm -Wall cpp_scatter.cpp -o cputest

and then run

time ./cputest

to benchmark on that machine. You could check out the old version of the code, run the benchmark, and then run the new one to get an idea of how much things slow down with the new ADP calculations. I think and hope it should add little, but it probably will slow down a tad.

Thanks for doing this!!

@tjlane
Copy link
Owner

tjlane commented Dec 3, 2017

Somehow I created a new branch instead of pushing to this PR. I guess upon further reading, I should have pushed to your fork. I thought I was doing that, but didn't work. Clearly my git-fu is rusty. I'll open a PR on that branch and see how Travis likes it...

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.

2 participants