[ENH] Switch to Selcomps 2.5 #119

emdupre · 2018-08-23T00:46:24Z

Changes proposed in this pull request:

Closes fproj_arr is never used #89
Initial commit of the 2.5 selection criteria workflow

To Do:

Add initial documentation
Update testing

Initial commit of 2.5

[FIX] Finish merging selcomps v2.5

Selcomps 2.5

- Explicitly compare size of mmix to length of metric arrays in seldict - Remove calls to utils.andb that could easily be done with bitwise operators. Should be more readable than checks against a sum array.

[FIX] Fix errors in tedana workflow and selcomps

codecov · 2018-08-24T16:55:04Z

Codecov Report

Merging #119 into master will increase coverage by 4.2%.
The diff coverage is 2.75%.

@@            Coverage Diff            @@
##           master     #119     +/-   ##
=========================================
+ Coverage   49.72%   53.93%   +4.2%     
=========================================
  Files          32       32             
  Lines        2035     1867    -168     
=========================================
- Hits         1012     1007      -5     
+ Misses       1023      860    -163

Impacted Files	Coverage Δ
tedana/model/fit.py	`28.38% <0%> (ø)`	⬆️
tedana/selection/select_comps.py	`8.25% <3.12%> (+3.2%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7f04ff...ed0f9a5. Read the comment docs.

tsalo · 2018-08-24T17:03:01Z

I think that further documentation can wait until @handwerkerd has the time for it, but do you want to handle the testing in this PR or later?

emdupre · 2018-08-24T18:04:18Z

Yes, I think I'd prefer to handle testing in this PR -- it will make it quite large but I'm also uncomfortable merging in a non-passing PR and breaking CI for the rest of the codebase. WDYT ?

tsalo · 2018-08-24T18:12:05Z

That makes sense. 👍

codecov · 2018-10-30T22:16:30Z

Codecov Report

Merging #119 into master will increase coverage by 4.26%.
The diff coverage is 3.96%.

@@            Coverage Diff             @@
##           master     #119      +/-   ##
==========================================
+ Coverage    48.7%   52.96%   +4.26%     
==========================================
  Files          32       32              
  Lines        2080     1903     -177     
==========================================
- Hits         1013     1008       -5     
+ Misses       1067      895     -172

Impacted Files	Coverage Δ
tedana/selection/__init__.py	`100% <ø> (ø)`	⬆️
tedana/workflows/tedana.py	`12% <0%> (ø)`	⬆️
tedana/model/fit.py	`29.05% <0%> (-0.13%)`	⬇️
tedana/decomposition/eigendecomp.py	`10.81% <14.28%> (ø)`	⬆️
tedana/selection/select_comps.py	`5.18% <2.63%> (+1.53%)`	⬆️
tedana/selection/_utils.py	`14.89% <50%> (+1.33%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b74530...f4e0c36. Read the comment docs.

@tsalo

* Use pandas for component tables. * Write out MEPCA component maps. * Fix. * Update. * Fix tedpca selection. * Re-add duplication. * More cleanup. * Add decision information to ICA compatible. * Add metrics to PCA component table. * Undo some refactoring. * Edit test failed file. * Undo refactor. * Fix style issues. * Add pandas to requirements. * Update requirements. * Fix tests. * Update test just to test a thing. * Fix how writeresults is called. * Add orphans to ignored and revert integration test. * Clean up MEICA component table. * Address review comments and track reasons to ignore components. * midkreg --> midkrej. Whoops! * Address some review comments. - Add docstring for comptable output in selcomps. - “variance explained 2 (normalized)” —> “normalized variance explained” - “variance explained (normalized)” —> “normalized variance explained” * Split PCA off into a separate function and rename some variables * Remove writect function. * Remove writect from init file too. * Remove unused import. * Rename run_pca to run_svd and improve documentation. * Add MLEPCA citation. * Remove mlepca flag and consolidate SVD functions. * Change markdown to rst and add figures. * Update file name. * Update file name. * Update documentation. * Update outputs file. * Document derivatives. * Fix some RST formatting issues in README. * rename model.monoexponential to decay - tedana.model.fit_decay => tedana.decay.fit_decay - tedana.model.fit_decay_ts => tedana.decay.fit_decay_ts ref: ME-ICA#135 * rename model.combine to combine breaks: tedana.model.make_optcom not sure if this should first be depricated ref: ME-ICA#135 * move io and utils into separate modules ref: ME-ICA#135 * move utils.new_nii_like to io.new_nii_like ref: ME-ICA#135 * move utils.filewrite to io.filewrite ref: ME-ICA#135 * move utils.load_data to io.load_data ref: ME-ICA#135 * revert docs/api.rts to a534c1e the changes did not work correctly, Failed to import 'tedana.io': no module named tedana.io * fix: remove duplicate imports * Add notebooks and figures for pipeline description page. * Fix some formatting. * move io.gscontrol_mmix to utils suggested by @tsalo * update api with the changes * fix test failure circular dependency circular dependency between io and utils after gscontrol_mmix move to utils ref: 5889135 * Revert "fix test failure circular dependency" This reverts commit 2200cd8. * Revert "move io.gscontrol_mmix to utils" This reverts commit 5889135. * add io.gscontrol_mmix to api.rst * Update multi-echo.rst * Update multi-echo.rst * Update figures in walkthrough. * Add yellow_heart emoji to README. * Split README into rst (for site) and md (for GitHub). * Update requirements. * Fix updated requirements. * Revert changes. * Try changing the tests a bit. * Wow... I had the files switched. * Fix test. * Fix test again. * Revert changes to tests. * Update README.md Added notes for creating a conda environment for use with tedana Added links to dependencies * [FIX] Logging in tedana and t2smap Closes: ME-ICA#127 Changes proposed in this pull request: - use logging.basicConfig to fix logging level for tedana and t2smap see: https://docs.python.org/3.7/howto/logging.html#logging-from-multiple-modules * Update list of workflow outputs on RTD site. * Fix docstrings. * Add note!!! * Fix MMIX dimensions. * Remove unused import. * Re-run workflow and re-make figures. - Change example voxel to one more affected by T1c-GSR. - Use SVD+decision tree for TEDPCA instead of MLEPCA (because MLEPCA doesn’t whiten this dataset). - Turn off GSR. * Change precision of floats in output tables and change midk to rejected. * Address review comments * Remove verbose output table * Update filenames in workflow docstring. * Change component table float precision from %g to %.6f. * Link workflow to outputs page. * Add Support and Communication page to docs. * Update line lengths, remove sys admin information * Fix old link. * [DOC] Remove getting started section * fix style errors - E127 continuation line over-indented for visual indent - E128 continuation line under-indented for visual indent * fix filewrite call filewriter is moved to io * Hackily "fix" bugs in component selection and model fitting. * Use tables in v2.5 component selection. * Remove unused import. * Revert TEDICA reindexing. I’d prefer to sort by variance explained, but Kappa is good too. * Clean up how metrics are added to ICA compatible.

emdupre

This is a first pass -- a few initial questions from your PR, @tsalo ! Thanks so much for all of your work on this !!

emdupre · 2018-11-15T02:17:02Z

tedana/model/fit.py

@@ -88,6 +88,8 @@ def fitmodels_direct(catd, mmix, mask, t2s, t2s_full, tes, combmode, ref_img,
                             '({0}) does not match second dimension of '
                             't2s ({1})'.format(catd.shape[2], t2s.shape[1]))

+    mask = t2s != 0  # Override mask because problems


@tsalo could you clarify, here ?

Not really... this is a hacky way I was able to get around the fact that different arrays were being masked differently (i.e., either with mask or with t2s != 0) and then were being compared element-wise, which was raising errors. This is something we should dig into before merging this PR, but I'm not sure why the arrays were being masked differently to begin with, so I don't know if it's better to make everything with mask, with t2s != 0, or to change which comparisons are made.

tedana/selection/select_comps.py

emdupre · 2018-11-27T21:49:33Z

Currently hitting a snag where the tedana command is not recognized. I've added an SSH key to circle to try and check in on the environment, but it's not yet recognized. Hoping to pick back up as soon as it's live, but if anyone looks over the file and is able to quickly spot the error, please let me know !

.circleci/config.yml

emdupre · 2018-11-28T20:15:10Z

It's passing now-- WDYT, @ME-ICA/tedana-devs ? I'd argue to merge this now, cut a release, and move forward from here to improve everything :)

emdupre · 2018-11-28T21:44:09Z

OK, I'm going to merge this ! We can (and I'm sure will !) revisit going forward.

Thanks everyone ! 🎉

emdupre and others added 12 commits August 22, 2018 18:31

Initial commit of 2.5

a1cffba

Merge pull request #3 from emdupre/selcomps-2.5

f344349

Initial commit of 2.5

Refactor selcomps.

ad0b1fa

Add documentation.

b4610af

Fix spacing.

8095863

Merge pull request #15 from tsalo/selcomps

7d4ac90

[FIX] Finish merging selcomps v2.5

Merge pull request #4 from emdupre/selcomps-2.5

0f30920

Selcomps 2.5

Fix tedana workflow.

2784212

Remove unused variable in selcomps.

d76a45a

Further refactoring.

9d4d0fb

- Explicitly compare size of mmix to length of metric arrays in seldict - Remove calls to utils.andb that could easily be done with bitwise operators. Should be more readable than checks against a sum array.

Add note about confusing section of code.

7db535f

Merge pull request #16 from tsalo/selcomps

ed0f9a5

[FIX] Fix errors in tedana workflow and selcomps

tsalo mentioned this pull request Sep 27, 2018

[ENH] Track PCA and ICA component selection decisions #122

Merged

tsalo mentioned this pull request Oct 30, 2018

[FIX] Include ignored components in ME-DN T1c time series #125

Merged

Merge branch 'v2.5' into selcomps-2.5

46f7f60

KirstieJane added the enhancement issues describing possible enhancements to the project label Oct 31, 2018

KirstieJane added this to the transparent and reproducible processing milestone Oct 31, 2018

This was referenced Nov 5, 2018

[ENH] Rename modules #136

Merged

A few updates to the roadmap and contributing guidelines emdupre/tedana#18

Merged

How much time will it take to have denoising working reliably? #153

Closed

tsalo and others added 2 commits November 14, 2018 20:40

Merge remote-tracking branch 'upstream/master' into selcomps-2.5

c635022

emdupre force-pushed the selcomps-2.5 branch from bc42daf to c635022 Compare November 15, 2018 02:05

emdupre commented Nov 15, 2018

View reviewed changes

[fix] rename get_data to build

9777155

[fix] try trick from nilearn

97aff12

emdupre force-pushed the selcomps-2.5 branch from 97f1c9f to 97aff12 Compare November 28, 2018 01:33

emdupre added 2 commits November 27, 2018 20:51

[fix] reactivate conda env

adb6901

[fix] update five echo testing

84adc5f

tsalo reviewed Nov 28, 2018

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

tsalo reviewed Nov 28, 2018

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

emdupre added 4 commits November 28, 2018 09:36

[fix] standardize dir names

8ad63e0

[fix] patch sed command based on @tsalo rec

1a3619a

[fix] update sorting in tedana-out, add caching

a586a61

[fix] add path to save cache

6958c59

emdupre force-pushed the selcomps-2.5 branch 2 times, most recently from 0467d90 to ae046a7 Compare November 28, 2018 18:28

[fix] reformat save_cache steps

e916154

emdupre force-pushed the selcomps-2.5 branch 2 times, most recently from 98d124f to ef00ee8 Compare November 28, 2018 18:44

[fix] rename caches

d29b156

emdupre force-pushed the selcomps-2.5 branch from ef00ee8 to d29b156 Compare November 28, 2018 18:47

[fix] patch sorting order

f4e0c36

emdupre merged commit 63d643a into ME-ICA:master Nov 28, 2018

tsalo mentioned this pull request Dec 2, 2018

Track selection criteria provenance across workflows #58

Closed

tsalo mentioned this pull request Jan 6, 2019

Eliminate Mask Argument in fitmodels_direct #175

Closed

1 task

tsalo added the output-change label Nov 20, 2020

tsalo mentioned this pull request Nov 20, 2020

Examine differences in results between original MEICA & current tedana #622

Open

jbteves added breaking change WIll make a non-trivial change to outputs and removed output-change labels Apr 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENH] Switch to Selcomps 2.5 #119

[ENH] Switch to Selcomps 2.5 #119

emdupre commented Aug 23, 2018 •

edited

Loading

codecov bot commented Aug 24, 2018

tsalo commented Aug 24, 2018

emdupre commented Aug 24, 2018

tsalo commented Aug 24, 2018

codecov bot commented Oct 30, 2018 •

edited

Loading

emdupre left a comment

emdupre Nov 15, 2018

tsalo Nov 15, 2018

emdupre commented Nov 27, 2018

emdupre commented Nov 28, 2018

emdupre commented Nov 28, 2018

[ENH] Switch to Selcomps 2.5 #119

[ENH] Switch to Selcomps 2.5 #119

Conversation

emdupre commented Aug 23, 2018 • edited Loading

codecov bot commented Aug 24, 2018

Codecov Report

tsalo commented Aug 24, 2018

emdupre commented Aug 24, 2018

tsalo commented Aug 24, 2018

codecov bot commented Oct 30, 2018 • edited Loading

Codecov Report

emdupre left a comment

Choose a reason for hiding this comment

emdupre Nov 15, 2018

Choose a reason for hiding this comment

tsalo Nov 15, 2018

Choose a reason for hiding this comment

emdupre commented Nov 27, 2018

emdupre commented Nov 28, 2018

emdupre commented Nov 28, 2018

emdupre commented Aug 23, 2018 •

edited

Loading

codecov bot commented Oct 30, 2018 •

edited

Loading