Skip to content

Set default value for half_block = 0 when reading ROOT files#1012

Merged
KrisThielemans merged 2 commits intoUCL:masterfrom
robbietuk:ROOT/InitialiseHalfBlock
Mar 27, 2022
Merged

Set default value for half_block = 0 when reading ROOT files#1012
KrisThielemans merged 2 commits intoUCL:masterfrom
robbietuk:ROOT/InitialiseHalfBlock

Conversation

@robbietuk
Copy link
Copy Markdown
Collaborator

@robbietuk robbietuk commented Mar 25, 2022

Fix for #1008?

@robbietuk
Copy link
Copy Markdown
Collaborator Author

robbietuk commented Mar 25, 2022

I am reluctant to remove half_block yet and do that in reponse to #928. Looking into that more now

@KrisThielemans
Copy link
Copy Markdown
Collaborator

KrisThielemans commented Mar 25, 2022

I suggest to have an option for compatibility with STIR 4, setting it to the old version. We should have given people notice about this change, so this way they can at least keep the same results. Obviously I'd default it to off (i.e. half_block=0). Not sure how to call it. feel free to suggest something

@robbietuk
Copy link
Copy Markdown
Collaborator Author

robbietuk commented Mar 25, 2022

Yes. However, it still needs a test because now I am concerned about rotations.

@KrisThielemans
Copy link
Copy Markdown
Collaborator

KrisThielemans commented Mar 25, 2022

only now? 😄

@KrisThielemans KrisThielemans linked an issue Mar 25, 2022 that may be closed by this pull request
Uninitialised value in ROOT processing causes a segfault in LmToProjData::process_data() #1008
Closed
@KrisThielemans KrisThielemans added this to the v5.0.1 milestone Mar 25, 2022
@robbietuk robbietuk closed this Mar 25, 2022
@robbietuk robbietuk force-pushed the ROOT/InitialiseHalfBlock branch from 5e0364c to 06a68d1 Compare March 25, 2022 21:18
@robbietuk robbietuk reopened this Mar 25, 2022
robbietuk
robbietuk commented Mar 25, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator Author

@robbietuk robbietuk left a comment

Choose a reason for hiding this comment

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

Forgot to commit before a forcepush... Apparently that closes the PR, who knew.

Comment thread CMakeLists.txt Outdated
Comment thread src/IO/InputStreamFromROOTFileForCylindricalPET.cxx
@robbietuk robbietuk force-pushed the ROOT/InitialiseHalfBlock branch from f43097c to 555ee63 Compare March 27, 2022 16:17
@robbietuk
Copy link
Copy Markdown
Collaborator Author

robbietuk commented Mar 27, 2022

Changed STIR_ROOT_AS_V4 to STIR_ROOT_ROTATION_AS_V4 as it will be more useful later. I think this is ready to go and we can test later? It will likely mean that a number of older GATE geometries are now rotated in version5 but we are going to fix that later?

KrisThielemans
KrisThielemans reviewed Mar 27, 2022
View reviewed changes
Comment thread documentation/release_5.0.htm Outdated
@KrisThielemans KrisThielemans changed the title Hotfix - Set default value for half_block = 0 Set default value for half_block = 0 when reading ROOT files Mar 27, 2022
@KrisThielemans KrisThielemans merged commit ec0db72 into UCL:master Mar 27, 2022
This was referenced Mar 27, 2022
Closed
Closed
@robbietuk robbietuk deleted the ROOT/InitialiseHalfBlock branch March 27, 2022 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@KrisThielemans KrisThielemans KrisThielemans left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v5.0.1

Development

Successfully merging this pull request may close these issues.

Uninitialised value in ROOT processing causes a segfault in LmToProjData::process_data()

2 participants

@robbietuk @KrisThielemans