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

Adding gridsearch with parser to pysap #46

Merged
merged 21 commits into from
Jul 2, 2018

Conversation

bensarthou
Copy link
Contributor

WARNING: This code must be reviewed after PR #22 of Modopt package have been accepted. Also, it needs some data in .local/share/pysap, which can't be download for now from ftp.

Modifications

Thoses changes allows to use a gridsearch wrapper (in base/gridsearch) to multi-thread reconstructions on several parameters (passed as arrays) and also compute multiple metrics at once.

A working example is set in examples/ dir, it is really the main scripts of study_launcher.py and post_processing.py concatenated in one file. Read README in gridsearch plugin to see how it works.

Quickly, the principle is that study_launcher.py parses the config.ini file, with lists of parameters and metrics to try on the gridsearch. Then, the gridsearch is called on one of the reconstruction function. Results are stored in results/ folder and the post_processing.py script reads it, and compute statistics over it.

@coveralls
Copy link

coveralls commented Apr 13, 2018

Pull Request Test Coverage Report for Build 130

  • 9 of 717 (1.26%) changed or added relevant lines in 14 files are covered.
  • 9 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-12.6%) to 43.263%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pysap/plugins/mri/reconstruct/reconstruct.py 0 2 0.0%
pysap/base/transform.py 3 5 60.0%
pysap/extensions/transform.py 1 4 25.0%
pysap/init.py 0 12 0.0%
pysap/base/gridsearch.py 0 31 0.0%
pysap/extensions/formating.py 0 32 0.0%
pysap/plugins/mri/gridsearch/data.py 0 93 0.0%
pysap/plugins/mri/gridsearch/study_launcher.py 0 95 0.0%
pysap/plugins/mri/gridsearch/reconstruct_gridsearch.py 0 153 0.0%
pysap/plugins/mri/gridsearch/post_processing.py 0 285 0.0%
Files with Coverage Reduction New Missed Lines %
pysap/extensions/transform.py 2 74.19%
pysap/init.py 2 43.18%
pysap/data.py 5 40.71%
Totals Coverage Status
Change from base Build 110: -12.6%
Covered Lines: 1265
Relevant Lines: 2924

💛 - Coveralls

@sfarrens
Copy link
Contributor

Missing __init__.py in pysap/pysap/plugins/mri/gridsearch/

acc_factor=None):

ref = get_sample_data("mri-slice-nifti")
ref = _l2_normalize(ref.data)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should probably be

ref.data = _l2_normalize(ref.data)

info['mask_type'] = mask_type
info['acc_factor'] = acc_factor

return ref.astype("complex128"),\
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one also

return ref.data.astype("complex128"),\

# Sys import
import os.path as osp

import sys
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not used.


# Specific import
from pysap.plugins.mri.reconstruct.utils import convert_mask_to_locations
from pysap.plugins.mri.reconstruct.utils import convert_locations_to_mask
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also not used


_dirname_ = osp.dirname(osp.abspath(__file__))
_data_dirname_ = osp.join(_dirname_, "data")
DATADIR = osp.join(osp.expanduser("~"), ".local", "share", "pysap")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why you do not simply use the following?

_data_dirname_ = osp.join(osp.expanduser("~"), ".local", "share", "pysap")

import logging

import argparse
import itertools
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not used

import itertools

# Third party import
import requests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not used

# Third party import
import requests
import numpy as np
import matplotlib.pylab as plt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not used

@sfarrens sfarrens merged commit e7140ce into CEA-COSMIC:master Jul 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants