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

Typo in parallelproj source code #805

Closed
samdporter opened this issue Feb 1, 2023 · 6 comments
Closed

Typo in parallelproj source code #805

samdporter opened this issue Feb 1, 2023 · 6 comments

Comments

@samdporter
Copy link

samdporter commented Feb 1, 2023

/devel/build/sources/parallelproj/c/include/parallelproj_c.h

line 284
typo: short n_tofbins;
correction: short n_tofbins,

I didn't know exactly where this came from in SIRF/STIR so didn't do a PR

Tested after this correction and SIRF builds fine on master branch

@samdporter samdporter changed the title Typo in parallelproj_c.h source code Typo in parallelproj source code Feb 1, 2023
@KrisThielemans
Copy link
Member

Hi. this file comes from parallelproj, specifically https://github.com/gschramm/parallelproj/blob/master/c/include/parallelproj_c.h. It looks fine there. Could you check versions etc?

@samdporter
Copy link
Author

Old version of parallelproj (v1.0.0)
Shall I close?

@KrisThielemans
Copy link
Member

did it give STIR compilation problems? Maybe only with the TOF branch? If not, we could raise the parallelproj minimum version in STIR's CMakeLists.txt, and even the SIRF-SuperBuild's version_config.cmake.

(I guess SIRF never sees parallelproj itself, so not sure if this issue was in the right place).

@samdporter
Copy link
Author

Yes, it gave STIR compilation problems on the master branch

@KrisThielemans
Copy link
Member

ok. Please check in which version of parallelproj this was fixed and generate a PR on STIR at https://github.com/UCL/STIR/blob/ea105a0ce2299afe2e598f249ecbe69de988b7c2/CMakeLists.txt#L200 accordingly and the SIRF-SuperBuild at

set(parallelproj_REQUIRED_VERSION "1.0")
.

I'll transfer this issue

@KrisThielemans
Copy link
Member

This was fixed in parallelproj 1.0.1. STIR now requires that. I think we can just close this here.

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