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

Documenting tof branch merge conflicts #943

Closed
ashgillman opened this issue Oct 14, 2021 · 13 comments
Closed

Documenting tof branch merge conflicts #943

ashgillman opened this issue Oct 14, 2021 · 13 comments

Comments

@ashgillman
Copy link
Collaborator

ML_norm.cxx

image
image

Chosen strategy:
Keep the functions

@ashgillman
Copy link
Collaborator Author

ProjDataInfoCylindricalNoArcCorr.cxx

image

Chosen strategy:
It seems ToF adds some extra swapping for rings and detectors. Respect this, while allowing updated to_0)2pi stuff:

image

@ashgillman
Copy link
Collaborator Author

Scanner.cxx

image

Related to #181, 8b64889

@KrisThielemans not confident on this one, but it seems just to keep the master version?

image

@KrisThielemans
Copy link
Collaborator

MLnorm.cxx
Chosen strategy:
Keep the functions

incorrect. set_fan_data has been superseded by set_fan_data_add_gaps and similar with make_fan_data_remove_gaps. The modification is at the moment just the check if is_tof_data(), then call error

@KrisThielemans
Copy link
Collaborator

ProjDataInfoCylindricalNoArcCorr.cxx
Chosen strategy:
It seems ToF adds some extra swapping for rings and detectors. Respect this, while allowing updated to_0)2pi stuff

Agreed. let's do that in this merge (and the get_psi_offset() of course). I don't think we want any extra swapping, but we don't want the merge to affect the actual working of the TOF code.

@KrisThielemans
Copy link
Collaborator

Scanner.cxx
not confident on this one, but it seems just to keep the master version?

There certainly shouldn't be any changes in list_all_names and get_names_of_predefined_scanners w.r.t. master.

@KrisThielemans
Copy link
Collaborator

Anything else?

@ashgillman
Copy link
Collaborator Author

MLnorm.cxx
Chosen strategy:
Keep the functions

incorrect. set_fan_data has been superseded by set_fan_data_add_gaps and similar with make_fan_data_remove_gaps. The modification is at the moment just the check if is_tof_data(), then call error

Removed

@ashgillman
Copy link
Collaborator Author

Bin.h

image

Resolution:
Keep both

image

@ashgillman
Copy link
Collaborator Author

Bin.inl

image

Resolution:
We're happy to not have a constructor for time_frame? Just add time_frame(1) to each

@ashgillman
Copy link
Collaborator Author

Scanner.h

image

Resolution:
Just looks more complicated because of whitespace chages. Need to add max_num_of_timing_poss, size_timing_pos, timing_resolution to master.
Unfortunately, the order chosen means no more default values are possible for energy parameters - is this okay?

image

@ashgillman
Copy link
Collaborator Author

ProjDataInfoCylindrical.h

image

Resolution:
Keep both

image

@KrisThielemans
Copy link
Collaborator

Scanner.h
Resolution:
Just looks more complicated because of whitespace chages. Need to add max_num_of_timing_poss, size_timing_pos,
timing_resolution to master.
Unfortunately, the order chosen means no more default values are possible for energy parameters - is this okay?

can't spot the difference. Can you do a comparison without white-space and show that?

The Scanner constructor etc is a mess. Too many parameters!

@KrisThielemans
Copy link
Collaborator

Obsolete

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