Draft MEDIC dynamic distortion correction method (second attempt) #438

tsalo · 2024-05-04T20:14:09Z

Closes #36. An alternative to #435 that installs MEDIC as a dependency instead of implementing the whole tool as a series of interfaces and workflows.

Changes proposed:

Install Julia in the Dockerfile.
Add warpkit to dependencies.
Add a MEDIC interface.
Add a workflow to run MEDIC on a list of magnitude files and a list of phase files.

Dockerfile

tsalo · 2024-05-04T21:15:23Z

I cannot figure out how to get the FieldmapWrangler to find MEDIC-style setups (i.e., complex-valued, multi-echo BOLD scans). 😕

effigies · 2024-10-04T19:33:34Z

@tsalo Around L508 in your current wrangler.py, I think you want to do something like:

        medic_entities = {**base_entities, **{'part': 'mag'}}
        has_magnitude = tuple()
        with suppress(ValueError):
            has_magnitude = layout.get(
                suffix='bold',
                **medic_entities,
            )
        for mag_img in has_magnitude:
            phase_img = layout.get(**{**mag_img.get_entities(), **{'part': 'phase'}})
            if not phase_img:
                continue
            phase_img = phase_img[
            try:
                e = fm.FieldmapEstimation(
                    [
                        fm.FieldmapFile(mag_img.path, metadata=mag_img.get_metadata()),
                        fm.FieldmapFile(phase_img.path, metadata=phase_img.get_metadata()),
                    ]
                )
            except (ValueError, TypeError) as err:
                _log_debug_estimator_fail(
                    logger, "potential MEDIC fieldmap", [mag_img, phase_img], layout.root, str(err)
                )
            else:
                _log_debug_estimation(logger, e, layout.root)
                estimators.append(e)

codecov · 2024-10-04T19:54:33Z

Codecov Report

Attention: Patch coverage is 91.15044% with 10 lines in your changes missing coverage. Please review.

Project coverage is 74.95%. Comparing base (656a51d) to head (085670d).

Files with missing lines	Patch %	Lines
sdcflows/workflows/fit/medic.py	76.00%	6 Missing ⚠️
sdcflows/fieldmaps.py	90.47%	1 Missing and 1 partial ⚠️
sdcflows/utils/wrangler.py	88.23%	2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (656a51d) and HEAD (085670d). Click for more details.

HEAD has 57 uploads less than BASE

Flag BASE (656a51d) HEAD (085670d)

60 3

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #438      +/-   ##
==========================================
- Coverage   83.75%   74.95%   -8.80%     
==========================================
  Files          32       33       +1     
  Lines        2831     2943     +112     
  Branches      381      294      -87     
==========================================
- Hits         2371     2206     -165     
- Misses        390      679     +289     
+ Partials       70       58      -12

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

tsalo · 2024-10-04T20:00:52Z

Thanks @effigies!

sdcflows/utils/wrangler.py

Co-authored-by: Chris Markiewicz <[email protected]>

tsalo · 2024-10-04T20:39:26Z

I created a MEDIC-compliant test dataset (dsD), but it seems like the tests in test_wrangler use skeletons. Should I drop the test dataset and just generate a skeleton in the test file?

effigies · 2024-10-04T20:51:25Z

sdcflows/cli/tests/test_find_estimators.py

    import sdcflows.config as sc

    # Reload is necessary to clean-up the layout config between parameterized runs
    reload(sc)

    path = (tmp_path / test_id).absolute()
    generate_bids_skeleton(path, config)
-    with pytest.raises(SystemExit) as wrapped_exit:
+    # This was set to raise a SystemExit, but was only raising an ImageFileError
+    with pytest.raises(ImageFileError) as wrapped_exit:


This was probably expecting valid headers in the images.

I reverted the changes I made to this test since other PRs don't seem to have an issue. I'm not sure why the problem is happening in this PR only, but I've tried to remove any extraneous changes in case those were causing the problem.

All I can think is that maybe adding Julia and warpkit to the dependencies is affecting which versions of other dependencies are installed (e.g., a newer version of nibabel?).

tsalo · 2024-10-23T21:05:11Z

@effigies I finally got the tests passing. Do you know what my next steps should be?

tsalo · 2024-10-23T21:05:31Z

sdcflows/cli/tests/test_find_estimators.py

-def test_cli_finder_wrapper(tmp_path, capsys, test_id, config, estimator_id):
+def _test_cli_finder_wrapper(tmp_path, capsys, test_id, config, estimator_id):


Renamed to skip the test since it was failing.

tsalo · 2024-12-10T16:07:24Z

@effigies I was thinking I could create a reduced version of one of the runs in https://openneuro.org/datasets/ds005250 (maybe like 20 volumes?), but I don't have access to https://github.com/nipreps-data to push it. I'm also not sure if any of the tests actually run the distortion correction on real data.

tsalo · 2024-12-12T14:55:12Z

I pushed a single run with 50 volumes to https://gin.g-node.org/tsalo/ds005250-sdcflows. We can transfer ownership or make a fork in the nipreps-data organization. The dataset is 724 MB. Is that reasonable for a test dataset? I can reduce it further if necessary.

EDIT: I dropped them down to 10 volumes and also created a duplicate second session with different metadata.

effigies · 2024-12-12T15:15:43Z

Invited you to nipreps-data.

tsalo · 2024-12-12T15:28:42Z

Thanks! I'll create a GitHub repo for the datalad dataset and incorporate that into the tests.

tsalo · 2024-12-12T16:41:34Z

.github/workflows/build-test-publish.yml

+        # ds005250
+        datalad install -r https://gin.g-node.org/tsalo/ds005250-sdcflows.git
+        datalad update -r --merge -d ds005250-sdcflows/
+        datalad get -r -J 2 -d ds005250-sdcflows/ ds005250-sdcflows/sub-04/ses-2/


I set up ses-2 with IntendedFor and ses-1 with B0Field fields. The B0Fields aren't working.

Also, it looks like BIDSLayout.get_B0FieldIdentifiers doesn't work at all.

are your B0Field* lists? see bids-standard/pybids#684

They are! I didn't realize that was a known issue. I can modify my test dataset to just have strings for B0FieldIdentifier.

tsalo added 5 commits May 4, 2024 15:42

Add warpkit to dependencies.

50b5933

Write workflow.

ba9a65a

Update medic.py

49f9f90

Update medic.py

92342b5

Add MEDIC test dataset.

1468ca5

effigies reviewed May 4, 2024

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

tsalo added 2 commits May 4, 2024 16:57

Add TotalReadoutTime field.

27d0139

Install Julia with micromamba, not Docker.

e3a5f99

Aspirational doctest.

662542f

tsalo mentioned this pull request May 6, 2024

Draft MEDIC dynamic distortion correction method #435

Closed

tsalo added 5 commits May 7, 2024 11:41

Try wrangling MEDIC files.

63aa532

Merge remote-tracking branch 'upstream/master' into medic-whole

5f84bd2

Merge branch 'master' into medic-whole

5b9814a

Merge remote-tracking branch 'upstream/master' into medic-whole

7750eaa

Fix connections.

e63cd84

Merge branch 'master' into medic-whole

d2569da

Try @effigies' recommendation.

b535b5f

effigies reviewed Oct 4, 2024

View reviewed changes

sdcflows/utils/wrangler.py Outdated Show resolved Hide resolved

effigies reviewed Oct 4, 2024

View reviewed changes

sdcflows/utils/wrangler.py Outdated Show resolved Hide resolved

tsalo and others added 2 commits October 4, 2024 16:15

Update sdcflows/utils/wrangler.py

496fa10

Co-authored-by: Chris Markiewicz <[email protected]>

Search for phase to reduce false positives.

79d1fb1

tsalo added 3 commits October 4, 2024 16:42

Address style issues and fix error.

544ae7d

Update test_find_estimators.py

8bef670

Update test_find_estimators.py

b164fa3

effigies reviewed Oct 4, 2024

View reviewed changes

tsalo added 10 commits October 12, 2024 13:12

Update test_wrangler.py

2b9ff81

Add B0Field fields to mock dataset.

084f199

Replace B0Field with IntendedFor.

9f2c089

Update wrangler.py

bb660d2

Add second session to test spec.

c2205e1

It needs a third session!

670ebc8

Alright maybe this'll work?

9c5fdd8

Fix?

d7a59ee

Ignore long lines in test files.

14e1ff1

Fix long line.

1ba3e94

tsalo commented Oct 23, 2024

View reviewed changes

Change Julia version to make build work.

45b256a

Add test for new function.

5d13425

tsalo added 3 commits December 12, 2024 11:36

Test collection on real data.

3f48d93

Fix check.

2815590

Drop B0Field fields from fmaps.

ee8c38a

tsalo commented Dec 12, 2024

View reviewed changes

tsalo added 8 commits December 12, 2024 12:23

Update build-test-publish.yml

f54533b

Update wrangler.py

9c58221

Fix key.

e205348

Draft medic workflow test.

8f54fad

Fix paths to metadata files.

25caaf3

Run ruff.

54d92c4

Test both types of annotation.

7c6cc93

Update wrangler.py

085670d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draft MEDIC dynamic distortion correction method (second attempt) #438

Draft MEDIC dynamic distortion correction method (second attempt) #438

tsalo commented May 4, 2024 •

edited

Loading

tsalo commented May 4, 2024

effigies commented Oct 4, 2024

codecov bot commented Oct 4, 2024 •

edited

Loading

tsalo commented Oct 4, 2024

tsalo commented Oct 4, 2024

effigies Oct 4, 2024

tsalo Oct 12, 2024

tsalo Oct 12, 2024

tsalo commented Oct 23, 2024

tsalo Oct 23, 2024

tsalo commented Dec 10, 2024

tsalo commented Dec 12, 2024 •

edited

Loading

effigies commented Dec 12, 2024

tsalo commented Dec 12, 2024

tsalo Dec 12, 2024

tsalo Dec 12, 2024

bpinsard Dec 12, 2024

tsalo Dec 12, 2024

		def test_cli_finder_wrapper(tmp_path, capsys, test_id, config, estimator_id):
		def _test_cli_finder_wrapper(tmp_path, capsys, test_id, config, estimator_id):

Draft MEDIC dynamic distortion correction method (second attempt) #438

Are you sure you want to change the base?

Draft MEDIC dynamic distortion correction method (second attempt) #438

Conversation

tsalo commented May 4, 2024 • edited Loading

tsalo commented May 4, 2024

effigies commented Oct 4, 2024

codecov bot commented Oct 4, 2024 • edited Loading

Codecov Report

tsalo commented Oct 4, 2024

tsalo commented Oct 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsalo commented Oct 23, 2024

Choose a reason for hiding this comment

tsalo commented Dec 10, 2024

tsalo commented Dec 12, 2024 • edited Loading

effigies commented Dec 12, 2024

tsalo commented Dec 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsalo commented May 4, 2024 •

edited

Loading

codecov bot commented Oct 4, 2024 •

edited

Loading

tsalo commented Dec 12, 2024 •

edited

Loading