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

Limit BASIL --slicedt argument to ascending slice orders #348

Merged
merged 6 commits into from
Dec 1, 2023

Conversation

tsalo
Copy link
Member

@tsalo tsalo commented Nov 27, 2023

Closes #347.

Changes proposed in this pull request

  • Only use the --slicedt argument in BASIL calls when the slice order is ascending.
    • I wonder if this could still be missing some cases (e.g., slice encoding direction that isn't k).
  • Add plds as an output to the ComputeCBF interface. This will only be written out when the PLDs are slice-timing corrected.

@tsalo tsalo added the bug Something isn't working label Nov 27, 2023
@tsalo
Copy link
Member Author

tsalo commented Nov 28, 2023

I need to check that the PLD shift is correctly applied to NKI data.

@codecov-commenter
Copy link

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (3a34405) 62.77% compared to head (ed0a535) 62.80%.

Files Patch % Lines
aslprep/workflows/asl/cbf.py 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #348      +/-   ##
==========================================
+ Coverage   62.77%   62.80%   +0.02%     
==========================================
  Files          56       56              
  Lines        5177     5186       +9     
  Branches      721      722       +1     
==========================================
+ Hits         3250     3257       +7     
- Misses       1675     1676       +1     
- Partials      252      253       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tsalo
Copy link
Member Author

tsalo commented Dec 1, 2023

I was able to successfully run this branch on NKI data, so I'm going to merge.

@tsalo tsalo merged commit 10fc6ba into PennLINC:main Dec 1, 2023
20 checks passed
@tsalo tsalo deleted the fix-slicetiming branch December 1, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BASIL call does not account for irregular slice times
2 participants