Skip to content

superblocks-case update in find_ML_normfactors3D.cxx and apply_normfactors3D.cxx #3

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

Closed
wants to merge 2 commits into from

Conversation

emikhay
Copy link

@emikhay emikhay commented Oct 6, 2021

I copied find_ML_normfactors3D.cxx and apply_normfactors3D.cxx from UCL/STIR and then changed the num_physical_transaxial_crystals_per_block and num_physical_taxial_crystals_per_block calculation to "superblocks".

@emikhay
Copy link
Author

emikhay commented Oct 6, 2021

@KrisThielemans
Copy link

sadly I don't think this can work. it'll probably need corresponding changes in MLnorm.cxx and possibly elsewhere. Also, copying lots of changes from another branch by hand removes appropriate attribution to the original author in git.

My feeling is that we'll either need to do this as a PR on master and then merge master here, or merge master here and then do a PR here. I prefer the first option, as it makes the change more to the norm code more visible (as it doesn't have anything to do with blocksoncylindrical really) .

Of course, this assumes that we can merge master here. Wait for #2 to be merged and then attempt that?

KrisThielemans pushed a commit that referenced this pull request Nov 10, 2021
* fix pre-commit

* add CI test
@KrisThielemans
Copy link

This is now obsolete thanks to PR to UCL/master

@KrisThielemans
Copy link

Please close

@KrisThielemans
Copy link

@emikhay please close this one

@emikhay emikhay closed this Jan 27, 2022
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