Closed chuckie82 closed 7 years ago
tjlane
commented
7 years ago Maybe I don't fully understand: what specifically is the difference between/intended use of coffset and clen? I had been thinking of these as interchangeable -- where ultimately they just get added together.
@chuckie82 can you propose an interface & output behavior that would work well?
chuckie82
commented
7 years ago I would propose in translate.py: write_cspad_crystfel(detector, filename, coffset, intensity_file_type='cheetah', pixel_size=109.92) write_psana() also would need to change.
If you like I can send a PR and then we can all review.
tjlane
commented
7 years ago PR definitely preferred.
These changes seem good. My one preference would be to keep sane defaults whenever possible -- could we have coffset be a kwarg w/default 0?
tjlane
commented
7 years ago Maybe I don't fully understand: what specifically is the difference between coffset and clen? I had been thinking of these as interchangeable -- where ultimately they just get added together.
Can you propose an interface & output behavior that would work well?
Sent from my iPhone
On Oct 25, 2016, at 3:59 PM, chuckie82 notifications@github.com wrote:
psana geometry does not contain any information about coffset or clen. It only knows about the detector distance (IP).
to_crystfel_file() method in Cspad class writes the detector distance to coffset. It treats clen as a header.
In short, there's fundamentally not enough information to go from psana geometry to crystfel geom file. Solution: to_crystfel_file() needs a user argument, coffset or clen.
@tjlane, @chrisvam, @msdubrovin
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
chuckie82
commented
7 years ago coffset = detectorDistance - clen coffset: home to sample detectorDistance: detector to sample clen: detector to home
If you set clen to zero, it's technically wrong but kind of works. It will confuse our users, so I don't think we should do this.
chrisvam
commented
7 years ago Hi TJ,
I was going to build a new release soon. Is there anything special I should do to get this stuff included?
chris
On Nov 3, 2016, at 7:37 AM, TJ Lane notifications@github.com<mailto:notifications@github.com> wrote:
Closed #16https://github.com/slaclab/psgeom/issues/16.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/slaclab/psgeom/issues/16#event-846469941, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AGoInb5444S8B0Iqy7bLoLy4JTHj2SVHks5q6fGvgaJpZM4KglFn.
tjlane
commented
7 years ago @chrisvam what is the timeline you were thinking about? @chuckie82 do you need this now or can it wait?
In principle we can install the "newdets" branch by pulling it off github and into the psana-svn psgeom repo. Before we do that, however, it would be good to at least track down the one test fail we found. It would also be nice to get the princeton and ePix100 cameras in.
I could reasonably finish that by Tues/Wed next week. But until Monday I must stay focused on beamtime proposals.
I'll leave it up to you guys: either pull down this branch, which has a good chance of working, or wait until mid-next week. Or both.
chrisvam
commented
7 years ago On Nov 3, 2016, at 10:44 AM, TJ Lane notifications@github.com<mailto:notifications@github.com> wrote:
I could reasonably finish that by Tues/Wed next week. But until Monday I must stay focused on beamtime proposals.
Absolutely. I’ll coordinate with Chuck and we’ll take care of it…
chris
psana geometry does not contain any information about coffset or clen. It only knows about the detector distance (IP).
to_crystfel_file() method in Cspad class writes the detector distance to coffset. It treats clen as a header.
In short, there's fundamentally not enough information to go from psana geometry to crystfel geom file. Solution: to_crystfel_file() needs a user argument, coffset or clen.
@tjlane, @chrisvam, @msdubrovin