brainlife / ezbids

A web service for semi-automated conversion of raw imaging data to BIDS
https://brainlife.io/ezbids
MIT License
26 stars 13 forks source link

[FIX] Address several PET bug issues #110

Closed dlevitas closed 8 months ago

dlevitas commented 8 months ago

Further addresses #92

This PR address several issues pertaining to PET BIDS conversion:

1). Detect pet/blood data (both json and corresponding tsv files) and separate metadata requirements from pet/pet data.

2). Convert pet/blood data into a BIDS-compliant dataset. 3). Accepts ECAT-formatted data (.v, .v.gz) 4). Utilizes parallel for file renaming (rare, but but otherwise incredibly slow when renaming thousands of files)

dlevitas commented 8 months ago

@bendhouseart would you mind reviewing the functionality introduced in this PR?

bendhouseart commented 8 months ago

Checked the following on my end:

setup success status msg/error ID
Phantom with the example spreadsheet fail failed to run bids -- code:1 /app/handler/convert.js:5 import { readFileSync, writeFileSync, openSync, writeSync, closeSync, lstatSync, unlinkSync, linkSync } from 'fs'; ^^^^^^ SyntaxError: Cannot use import statement outside a module at Object.compileFunction (node:vm:360:18) at wrapSafe (node:internal/modules/cjs/loader:1126:15) at Module._compile (node:internal/modules/cjs/loader:1162:27) at Object.Module._extensions..js (node:internal/modules/cjs/loader:1252:10) at Module.load (node:internal/modules/cjs/loader:1076:32) at Function.Module._load (node:internal/modules/cjs/loader:911:12) at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12) at node:internal/main/run_main_module:22:47 #65b830fb4c1a13a89093658e
Phantom with an "incorrect" spreadsheet, then correcting in metadata editor fail "status_msg": "failed to run bids -- code:1\n/app/handler/convert.js:5\nimport { readFileSync, writeFileSync, openSync, writeSync, closeSync, lstatSync, unlinkSync, linkSync } from 'fs';\n^^^^^^\n\nSyntaxError: Cannot use import statement outside a module\n at Object.compileFunction (node:vm:360 #65b82b354c1a13a890936463
Real data from one of our COX-2 Subjects fail timed out, forced quit #65b82e574c1a13a8909364ee

Not complete until each row is filled out*

bendhouseart commented 8 months ago

The real data seems to be getting stuck somewhere, it's been running several minutes and this is the state of the output dir:

preprocess.err  preprocess.log  sub-0

Could be the pax headers messing things up:

.
└── sub-09
    ├── anat
    └── pet
        ├── ses-01
        │   └── PaxHeaders.29559
        └── ses-02
            └── PaxHeaders.2255

or the fact that dicoms in ses-01 and ses-02 have the following format:

LASTNAME_FIRSTNAME.PT.PET_PET_BRAIN_DYN_TRACER_34MA_(ADULT).0004.4664.2020.06.28.12.23.29_

dlevitas commented 8 months ago

Are there any errors in the web browser console?

bendhouseart commented 8 months ago

Only at the finalize page see:

Screenshot 2024-01-29 at 7 35 47 PM

That said, we can look over the sessions in the /tmp/ezbids-workdir together if you like.

Updated table above, will play again tomorrow and try out some conversions without blood data/spreadsheets.

dlevitas commented 8 months ago

Just pushed some commits that should resolve the finalize page error

bendhouseart commented 8 months ago

Take #2 following changes added with commit 8ad5b63

setup success status msg/error ID
Phantom with the example spreadsheet PASS None #65b952b02e56eca23622cfc4
Phantom with an "incorrect" spreadsheet, then correcting in metadata editor PASS No complaints #65b9695805fb3d4fbcac9a5e
Real data from one of our COX-2 Subjects TIMEOUT still timing out, upload succeeds, but gets stuck on the inflating step 65b95b2f2e56eca23622d0a9

Not complete until each row is filled out*

bendhouseart commented 8 months ago
Screenshot 2024-01-30 at 2 52 58 PM
bendhouseart commented 8 months ago

One thing I did notice that the metadata menu/editor doesn't agree with the value in the spreadsheet. But it's reading in everything else correctly and producing an invalid output (that's what I wanted to see) as a result.

Screenshot 2024-01-30 at 2 59 11 PM Screenshot 2024-01-30 at 3 00 12 PM
dlevitas commented 8 months ago

One thing I did notice that the metadata menu/editor doesn't agree with the value in the spreadsheet. But it's reading in everything else correctly and producing an invalid output (that's what I wanted to see) as a result.

This is by design for MetaboliteRecoverCorrectionApplied. If that value is True and the corresponding tsv header column ("hplc_recovery_fractions") doesn't exist, ezBIDS changes the value to False. It's attempting to correct a mistake that, if left unchecked, would result in a bids-validator error later on.

bendhouseart commented 8 months ago

Got it, so I just tried to rerun the data that produced the above screenshots, but that when I do ezBIDS doesn't pick up on the petness of the files and run dcm2niix instead.

Screenshot 2024-01-30 at 3 19 27 PM

Which is weird.

dlevitas commented 8 months ago

Got it, so I just tried to rerun the data that produced the above screenshots, but that when I do ezBIDS doesn't pick up on the petness of the files and run dcm2niix instead.

You mean you uploaded the output from your first attempt, meaning a BIDS-compliant dataset, or something else? I'm a little unsure how to re-produce this issue.

bendhouseart commented 8 months ago

I reran it on the same data in the same folder. Then I made a duplicate of that data, and it produced the same error.

What I haven't done is to drag and drop data, I've been using the file browser instead.

bendhouseart commented 8 months ago

Also, I think the issue with it not picking up the pet files was because I had excel open, I would call this good to go for now and merge it.

dlevitas commented 8 months ago

I'm going to perform a few additional tests, but I'll plan to merge this PR by the end of the week.

dlevitas commented 8 months ago

@bendhouseart when you get the chance, would you mind looking over commit 64043f3?

I ended up adding ECAT support (via ecatpet2bids) and merged the functionality of find_dicomdir.py and find_petdir.py into the former and removed the latter. To test this, I uploaded a "dataset" containing 5 different types of data:

All files were successfully identified and transformed based on their datatype and format (i.e. MRI dicoms --> dcm2niix; PET dicoms --> dcm2niix4pet; PET ECAT --> ecatpet2bids).

The PET data I was able to find online is still rather small, so I haven't replicated your inflation error from before.

bendhouseart commented 8 months ago

Just tested the final (it's absolutely final i'm sure) iteration you pushed yesterday and it works flawlessly with our real data.

Screenshot 2024-02-06 at 11 07 55 AM

So long as users place the extra PET metadata spreadsheet along with the dicom files things go off without a hitch. One might argue that ezBIDS should be able to locate a PET metadata spreadsheet from anywhere, but I would call this done as the user has the interface to correct for missing metadata.

dlevitas commented 8 months ago

@bendhouseart I recall us discussing how ezBIDS should handle PET metadata spreadsheet(s), landing on users needing to place a spreadsheet in each corresponding raw data folder.

This approach is fine by me; however, I'm also amenable to a provenance framework (akin to BIDS) where all raw data folders or files underneath a spreadsheet are assumed to have that specified metadata. As an example:

-- pet_metadata_spreadsheet.xlsx
-- sub1_raw_data
---- run1_ecat.v
---- run2_ecat.v

Where, both ECAT-formatted raw PET data would be given the metadata in the folder above. If this isn't how PET metadata would be stored, or if it's outside the scope of the PR, we can ignore this.

bendhouseart commented 8 months ago

Out of scope!

What's going to be more helpful to the user, in the immediate, is them being clued in on how to format and place their PET spreadsheets. But, really I don't want to make any assumptions or optimizations until the current iteration gets tested on real users.