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

Fix memory and styling errors #307

Merged
merged 4 commits into from Feb 1, 2018

Conversation

Projects
None yet
3 participants
@vessemer
Copy link
Contributor

vessemer commented Jan 31, 2018

Previous behaviour leads to various memory errors. It happened time to time 'cause of unreasonably large choice of the DATA_SHAPE. The average maximum height of lungs for a grown man, according to this research (Table 2), is 282mm with std equals to 22mm and median 274mm (Table 2). Assuming normal distribution, 3 sigmas aside the mean will lead to the probability of case h belong to the interval P{|282 - h| <= 66} ~= 0.9974. Taking into an account the fact that all algorithms which were described (not only implemented ones) works with spacing on z-axis >= 0.9, therefore, having an upper bound value of (282 + 66) / 0.9 = 386 (voxels) for z-axis should be enough for all cases.

I've made DATA_SHAPE to be (512, 512, 512) which should fit in memory while having reserved space for the z-axis.

CLA

  • I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@vessemer vessemer referenced this pull request Jan 31, 2018

Closed

Chose an appropriate DATA_SHAPE #304

1 of 1 task complete

@reubano reubano referenced this pull request Jan 31, 2018

Merged

Reactivate tests and clear models #306

1 of 1 task complete
@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Feb 1, 2018

How does this compare to #307? :)

@reubano

This comment has been minimized.

Copy link
Contributor

reubano commented Feb 1, 2018

Failing. But at least not due to memory error :)

=================================== FAILURES ===================================
______________________________ test_segment_luna _______________________________
metaimage_path = '/images/LUNA-0001/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797/1.3.6.1.4.1.14519.5.2.1.6279.6001.102133688497886810253331438797.mhd'
luna_nodule = {'x': 0, 'y': 100, 'z': 556}
    def test_segment_luna(metaimage_path, luna_nodule):
>       predicted = predict(metaimage_path, [luna_nodule])
src/tests/test_segmentation.py:35: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
src/algorithms/segment/trained_model.py:51: in predict
    volumes = calculate_volume(segment_path, centroids)
src/algorithms/segment/trained_model.py:78: in calculate_volume
    labels = [mask[centroid['x'], centroid['y'], centroid['z']] for centroid in centroids]
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.0 = <list_iterator object at 0x7fab5f819fd0>
>   labels = [mask[centroid['x'], centroid['y'], centroid['z']] for centroid in centroids]
E   IndexError: index 556 is out of bounds for axis 2 with size 512
src/algorithms/segment/trained_model.py:78: IndexError
@reubano

This comment has been minimized.

Copy link
Contributor

reubano commented Feb 1, 2018

@lamby this one should superceed #304.

@reubano

This comment has been minimized.

Copy link
Contributor

reubano commented Feb 1, 2018

Based on this comment it seems like best option is to add to here, just the bits of code from #308 that will fix the current error... agree?

@vessemer

This comment has been minimized.

Copy link
Contributor

vessemer commented Feb 1, 2018

@reubano, done, but I've added all code from #308, 'cause in other cases, it'll lead to wrong tests for a new calculate_volumes which now works with real-world units.

@vessemer

This comment has been minimized.

Copy link
Contributor

vessemer commented Feb 1, 2018

============================= test session starts ==============================
platform linux -- Python 3.6.3, pytest-3.1.3, py-1.5.2, pluggy-0.4.0
rootdir: /app, inifile:
collected 63 items 
src/tests/test_classification.py ...
src/tests/test_classification_3dlrcnn.py ...
src/tests/test_cropping.py ...
src/tests/test_endpoints.py ..x.....
src/tests/test_generators.py .........
src/tests/test_grt123_preprocess.py ...
src/tests/test_identification.py xxx
src/tests/test_improved_segmentation.py ..
src/tests/test_loading.py .......
src/tests/test_preprocess_dicom.py ....
src/tests/test_segment_evaluate.py ........
src/tests/test_segmentation.py ....xxx
src/tests/test_volume_calculation.py ...
=========================== short test summary info ============================
XFAIL src/tests/test_endpoints.py::test_identify
  Test was stopped after timeout
XFAIL src/tests/test_identification.py::test_identify_nodules_001
  Test was stopped after timeout
XFAIL src/tests/test_identification.py::test_identify_nodules_003
  Test was stopped after timeout
XFAIL src/tests/test_identification.py::test_identify_luna
  Test was stopped after timeout
XFAIL src/tests/test_segmentation.py::test_nodule_segmentation
  Test was stopped after timeout
XFAIL src/tests/test_segmentation.py::test_lung_segmentation
  Test was stopped after timeout
XFAIL src/tests/test_segmentation.py::test_stop_timeout
  Test was stopped after timeout
=============================== warnings summary ===============================
src/tests/test_endpoints.py::test_identify
  /app/src/tests/../../src/preprocess/extract_lungs.py:34: RuntimeWarning: invalid value encountered in less
    truncate=2.0) < intensity_th
src/tests/test_identification.py::test_identify_nodules_001
  /app/src/tests/../../src/preprocess/extract_lungs.py:34: RuntimeWarning: invalid value encountered in less
    truncate=2.0) < intensity_th
src/tests/test_identification.py::test_identify_nodules_003
  /app/src/tests/../../src/preprocess/extract_lungs.py:34: RuntimeWarning: invalid value encountered in less
    truncate=2.0) < intensity_th
-- Docs: http://doc.pytest.org/en/latest/warnings.html
============== 56 passed, 7 xfailed, 3 warnings in 991.87 seconds ==============
@reubano

This comment has been minimized.

Copy link
Contributor

reubano commented Feb 1, 2018

Tests look good now (I think?). Just the styling that is making travis unhappy

$ flake8 prediction
prediction/src/algorithms/evaluation/evaluate_detection.py:233:80: W291 trailing whitespace
prediction/src/algorithms/evaluation/evaluate_detection.py:246:20: E999 SyntaxError: invalid syntax
The command "flake8 prediction" exited with 1.

$ pycodestyle prediction
prediction/src/algorithms/evaluation/evaluate_detection.py:233:80: W291 trailing whitespace
@vessemer

This comment has been minimized.

Copy link
Contributor

vessemer commented Feb 1, 2018

Yes, but this was fixed in PR #306

@reubano

This comment has been minimized.

Copy link
Contributor

reubano commented Feb 1, 2018

#306 should just be refactoring code. So it seems that the changes in evaluate_detection.py are actually fixes which would go here.

@reubano reubano changed the title Chose an appropriate DATA_SHAPE Correct memory and styling errors Feb 1, 2018

@reubano reubano changed the title Correct memory and styling errors Fix memory and styling errors Feb 1, 2018

@vessemer

This comment has been minimized.

Copy link
Contributor

vessemer commented Feb 1, 2018

@reubano, I'm a bit confused with all those permutations, but it seems to be done :)

@reubano

This comment has been minimized.

Copy link
Contributor

reubano commented Feb 1, 2018

Yes, sorry for all the back and forth. The main idea was to just include the relevant (and minimum set of) code changes needed to get the travis tests passing. It just so turned out that some things we didn't think were related to fixing the travis tests, were actually needed after-all.

@reubano reubano merged commit 661ae2f into drivendataorg:master Feb 1, 2018

2 checks passed

concept-to-clinic/cla @vessemer has signed the CLA.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@reubano reubano referenced this pull request Feb 1, 2018

Closed

Allows to calculate_volume in mm #308

1 of 1 task complete
@vessemer

This comment has been minimized.

Copy link
Contributor

vessemer commented Feb 1, 2018

Glad to help :)

@reubano

This comment has been minimized.

Copy link
Contributor

reubano commented Feb 1, 2018

Great work!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment