@Serhiy-Shekhovtsov

Signed up since Sept. 2, 2017

Points

Timestamp Points Contributor Ad-hoc References
Jan. 30, 2018 3 @Serhiy-Shekhovtsov No
Jan. 26, 2018 3 @Serhiy-Shekhovtsov No PR #290
Jan. 26, 2018 5 @Serhiy-Shekhovtsov No Issues #285
Jan. 26, 2018 2 @Serhiy-Shekhovtsov No Issues #150
Jan. 21, 2018 40 @Serhiy-Shekhovtsov No Issues #150
PR #291
Jan. 15, 2018 20 @Serhiy-Shekhovtsov No Issues #280
PR #286
Jan. 12, 2018 10 @Serhiy-Shekhovtsov No PR #272
Jan. 12, 2018 20 @Serhiy-Shekhovtsov No Issues #215
PR #269
Jan. 11, 2018 3 @Serhiy-Shekhovtsov No PR #284
Jan. 5, 2018 2 @Serhiy-Shekhovtsov No Issues #268
Jan. 5, 2018 5 @Serhiy-Shekhovtsov No PR #273
Jan. 1, 2018 5 @Serhiy-Shekhovtsov No Issues #240
PR #270
Dec. 28, 2017 3 @Serhiy-Shekhovtsov No PR #215
Dec. 21, 2017 2 @Serhiy-Shekhovtsov No Issues #145
Dec. 21, 2017 2 @Serhiy-Shekhovtsov No Issues #228
Dec. 20, 2017 2 @Serhiy-Shekhovtsov No Issues #202
Dec. 20, 2017 3 @Serhiy-Shekhovtsov No PR #225
Dec. 20, 2017 2 @Serhiy-Shekhovtsov No
Dec. 20, 2017 2 @Serhiy-Shekhovtsov No Issues #150
Dec. 20, 2017 2 @Serhiy-Shekhovtsov No Issues #215
Dec. 12, 2017 3 @Serhiy-Shekhovtsov No PR #260
Dec. 12, 2017 5 @Serhiy-Shekhovtsov No Issues #145
PR #233
Dec. 12, 2017 40 @Serhiy-Shekhovtsov No Issues #148
PR #250
Dec. 1, 2017 5 @Serhiy-Shekhovtsov No Issues #148
PR #254
Nov. 26, 2017 5 @Serhiy-Shekhovtsov No Issues #150
PR #249
Nov. 26, 2017 5 @Serhiy-Shekhovtsov No Issues #251
PR #235
Nov. 26, 2017 2 @Serhiy-Shekhovtsov No Issues #240
Nov. 23, 2017 8 @Serhiy-Shekhovtsov No Issues #148
PR #226
Nov. 21, 2017 8 @Serhiy-Shekhovtsov No Issues #148
PR #226
Nov. 18, 2017 8 @Serhiy-Shekhovtsov No Issues #228
PR #231
Nov. 15, 2017 3 @Serhiy-Shekhovtsov No Issues #148
PR #197
Nov. 8, 2017 3 @Serhiy-Shekhovtsov No Issues #191
PR #213
Oct. 16, 2017 2 @Serhiy-Shekhovtsov No Issues #145
Sept. 11, 2017 20 @Serhiy-Shekhovtsov No Issues #91
PR #95
Sept. 5, 2017 1 @Serhiy-Shekhovtsov No Issues #89
Sept. 4, 2017 3 @Serhiy-Shekhovtsov No PR #87

Activity

9 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on issue #150: Vue: implement 'Segmentation' UI component(s)

@wanghaisheng sorry, didn't get your question. With this tool you can select an area of any shape. Btw, this issue can be closed now. as it has been implemented in #291
9 months, 3 weeks ago

@Serhiy-Shekhovtsov opened a new pull request: #291: Annotate and segment - editing nodule area + DICOM navigation

#150: Implemented saving\loading selected nodule area. Image navigation is also supported. ### Loading\showing. By default first slice containing area is selected: ![image](https://i.imgur.com/ALU1AFu.gif) ### Image navigation support: ![image](https://i.imgur.com/7r1mBBB.gif) ### Saving\reseting selected area: ![image](https://i.imgur.com/nbi8W8X.gif) ### BONUS - copy selected area to next \ prev slice ![image](https://i.imgur.com/oa9vf1e.gif) ## CLA - [x] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
9 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on issue #268: Classification throws RuntimeError for real nodule location

@reubano the patch is, actually, invalid: > test is running on small DICOM image and 72 is far outside the boundaries of Z dimension(0, 28). The cropping will fail to create a patch of expected size (96, 96, 96). **The test can be fixed by simple replacement of `dicom_path_003`(small image) with `dicom_paths[2]`(full image)**. So, I have fixed that test by creating two new tests in [this commit](https://github.com/concept-to-clinic/concept-to-clinic/pull/272/commits/df2738277856e0eb6e3c00d4c8e92011f9d2b446).
9 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on issue #268: Classification throws RuntimeError for real nodule location

@reubano I can't reproduce it on latest version. Can you check it again, please?
9 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on issue #279: UI feature: add overall progress indicator ("wizard" element)

@louisgv I am not working on it, just like the feature :) Especially if the progress will be saved on loaded. This gives me pain every time I work on interface frontend. Have to go to first step and select the case each time the page is reloaded.
10 months ago

@Serhiy-Shekhovtsov opened a new pull request: #286: Sending and saving of nodule properties on Annotate screen

Fixed few bugs of Annotate screen. Fixed sending of the Nodule data to backend. Added fields for storing `probability_concerning` and `note`. Includes fix of #285 ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
10 months ago

@Serhiy-Shekhovtsov commented on issue #280: Annotation: per-nodule notes should actually be POSTed and saved

@isms, I can see that we are using `PATCH` for updating the candidate. Is there a reason for using `POST` here?
10 months ago

@Serhiy-Shekhovtsov created a new issue: #285: User should be able to run nodule candidates prediction

Currently we can only create a new case based on local images. There should be a button to start the actual prediction. After clicking it should show that prediction is running. Once prediction is ready - it should show update the status on the page. ## Checklist before submitting - [ ] I have confirmed this using the officially supported Docker Compose setup using the `local.yml` configuration and ensured that I built the containers again and they reflect the most recent version of the project at the `HEAD` commit on the `master` branch - [ ] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [ ] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [ ] I provided **screenshots** where appropriate - [ ] I filled out all the relevant sections of this template
10 months ago

@Serhiy-Shekhovtsov opened a new pull request: #284: Added test for multiple patches cropping

Added test for changes introduced in PR #273 - (fixed classification prediction for multiple nodules) ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
10 months ago

@Serhiy-Shekhovtsov commented on PR #272: Fixed coordinates scaling for classification prediction

@reubano, coordinates for this test has been hand crafted to match the real nodule on the third image: **LIDC-IDRI-0003**. But the `glob` method returns unsorted list. By coincidence on your machine the third item is **LIDC-IDRI-0002**. I will fix the test to sort the list. ![image](https://user-images.githubusercontent.com/607527/34689922-df2e5496-f4bf-11e7-99ce-7c36862d9a7f.png)
10 months, 1 week ago

@Serhiy-Shekhovtsov commented on PR #272: Fixed coordinates scaling for classification prediction

To be precise - the reported issue was caused by wrong coordinates. The coordinates specified in the test were good for full size DICOM but test was running on a small image. But I have found an other issue with classification. It was confirmed by @caseyfitz [here](https://github.com/concept-to-clinic/concept-to-clinic/pull/272#issuecomment-355413434). This PR includes tests for real nodules.
10 months, 1 week ago

@Serhiy-Shekhovtsov commented on PR #272: Fixed coordinates scaling for classification prediction

@reubano converting to boolean is a fix for an issue of lost `meta.spacing`. Now `params.spacing` is boolean and it tells if we need to scale the image. But it doesn't wipe out the `meta.spacing` anymore.
10 months, 1 week ago

@Serhiy-Shekhovtsov commented on issue #268: Classification throws RuntimeError for real nodule location

Coordinates stored in in nonscaled form. So we have to scale them. And for that we need to know the spacing. So the point of suggested change is - preserving properties of `meta` object.
10 months, 1 week ago
10 months, 1 week ago

@Serhiy-Shekhovtsov opened a new pull request: #273: fixed classification prediction for multiple nodules

Bug fixed. When the length of the nodules list is more then 1 – prediction fails as follows: ``` ~/concept-to-clinic/prediction/src/algorithms/classify/src/gtr123_model.py in predict(ct_path, nodule_list, model_path) 275 results = [] 276 --> 277 for nodule, (cropped_image, coords) in zip(nodule_list, patches): 278 cropped_image = Variable(torch.from_numpy(cropped_image[np.newaxis, np.newaxis]).float()) 279 cropped_image.volatile = True ValueError: too many values to unpack (expected 2) ``` More details [here](https://github.com/concept-to-clinic/concept-to-clinic/pull/272#issuecomment-355413434). ## CLA - [x] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well <!--- Provide a general summary of your changes in the Title above --> ## Description <!--- Describe your changes in detail --> ## Reference to official issue <!--- If fixing a bug, there should be an existing issue describing it with steps to reproduce --> <!--- Please link to the issue here: --> ## Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If adding a new feature or making improvements not already reflected in an official issue, please reference the relevant sections of the design doc --> ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## Screenshots (if appropriate): ## Metrics (if appropriate): If you submitting a PR for a prediction algorithm (segmentation, identification, or classification) please fill in values for as many as the below statistics as are relevant. *algorithms by metric* metric | relevant algorithms -------|-------------------- [accuracy <sup>1</sup> <sup>2</sup>](https://stats.stackexchange.com/a/231237/143678) | classification, identification [data IO](https://unix.stackexchange.com/questions/55212) | classification, identification, segmentation [Dice coefficient <sup>3</sup>](https://en.wikipedia.org/wiki/S%C3%B8rensen%E2%80%93Dice_coefficient) | segmentation [disk space usage](https://www.cyberciti.biz/faq/linux-check-disk-space-command) | classification, identification, segmentation [Hausdorff distance <sup>3</sup>](https://en.wikipedia.org/wiki/Hausdorff_distance) | segmentation [Jaccard index](https://en.wikipedia.org/wiki/Jaccard_index) | segmentation [Log Loss](http://wiki.fast.ai/index.php/Log_Loss) | classification, identification <sup>4</sup> [memory usage](https://stackoverflow.com/questions/110259) | classification, identification, segmentation [prediction time <sup>2</sup>](https://stackoverflow.com/questions/385408) | classification, identification, segmentation [sensitivity <sup>3</sup>](http://wiki.fast.ai/index.php/Deep_Learning_Glossary#Recall) | segmentation [specificity <sup>3</sup>](http://wiki.fast.ai/index.php/Deep_Learning_Glossary#Specificity) | segmentation [training time <sup>2</sup>](https://stackoverflow.com/questions/385408) | classification, identification, segmentation *notes* 1. Use 5-fold cross validation if there is enough time and computational power available, otherwise use a holdout set 1. This metric may be automatically calculated by the machine learning architecture, e.g., Keras 1. The calculations for these metrics [are available here](https://github.com/concept-to-clinic/concept-to-clinic/blob/master/prediction/src/algorithms/segment/src/evaluate.py) 1. In order to calculate Log Loss for identification, the data needs to be arranged in a way that shows for each pixel, whether or not it is a nodule centriod. Restated, the pixel level labels of 1/0 would correspond to centriod/not-centriod. *metrics by algorithm* algorithm | relevant metrics ---------------|------------------ classification | accuracy, data IO, disk space usage, Log Loss, memory usage, prediction time, training time identification | accuracy, data IO, disk space usage, Log Loss, memory usage, prediction time, training time segmentation | data IO, Dice coefficient, disk space usage, Hausdorff distance, Jaccard index, memory usage, prediction time, sensitivity, specificity, training time When reporting your values, please use a format similar to the following example. algorithm | metric | value -------------|--------|------: segmentation | accuracy | 99.5 segmentation | Jaccard index | 0.5 segmentation | prediction time (s) | 45.3 segmentation | memory usage (MB) | 5.4 ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
10 months, 1 week ago

@Serhiy-Shekhovtsov commented on PR #272: Fixed coordinates scaling for classification prediction

@caseyfitz turned out, one little `else` at [this line](https://github.com/Serhiy-Shekhovtsov/concept-to-clinic/blob/4887eeb6778013fce10e7e91fd846c84cb7cb248/prediction/src/preprocess/crop_patches.py#L68) can solve the issue with multiple centroids. I will push the fix and tests soon.
10 months, 1 week ago

@Serhiy-Shekhovtsov commented on PR #272: Fixed coordinates scaling for classification prediction

@caseyfitz, yes I saw this problem. Will try to solve it today. And also will improve the approach a bit and fix the failing tests.
10 months, 1 week ago

@Serhiy-Shekhovtsov commented on issue #268: Classification throws RuntimeError for real nodule location

In current implementation the preprocessing is overriding the `meta.spacing` making it's impossible to scale the coordinates. I am giving you the real example - when `params.spacing` is 1 and the `meta.spacing` is 2.5 we will zoom the image and then set `meta.spacing = 1`. The **2.5** value is lost after that and we cannot scale coordinates. Also, I can't see any usage for `params.spacing` except for [this](https://github.com/concept-to-clinic/concept-to-clinic/blob/666fb1bf410cbd80eb7928d0e3c5f2433ad668a5/prediction/src/algorithms/classify/src/gtr123_model.py#L261) one place, where it is 1, so it doesn't affect the scaling ratio.
10 months, 1 week ago

@Serhiy-Shekhovtsov commented on issue #268: Classification throws RuntimeError for real nodule location

@vessemer the problem we had, the zooming factor got lost after zooming. When `params.spacing` is 1 and the `meta.spacing` is 2.5 we will zoom the image and then wipe out the `meta.spacing`. Then we will not know how to zoom the coordinates. Also I noticed that there is no use for `params.spacing`, so I converted it boolean. When it's `true` we will zoom the image during preprocessing according to `meta.spacing` but we will also keep that value for future coordinates rescaling.
10 months, 1 week ago
10 months, 1 week ago

@Serhiy-Shekhovtsov commented on issue #268: Classification throws RuntimeError for real nodule location

@reubano, @WGierke something like [this](https://github.com/concept-to-clinic/concept-to-clinic/pull/272)?
10 months, 1 week ago

@Serhiy-Shekhovtsov opened a new pull request: #272: Fixed coordinates scaling for classification prediction

<!--- Provide a general summary of your changes in the Title above --> We had a problem - the preprocessing is [zooming the image](https://github.com/concept-to-clinic/concept-to-clinic/blob/666fb1bf410cbd80eb7928d0e3c5f2433ad668a5/prediction/src/preprocess/preprocess_ct.py#L144). In other words, the pixel matrix is resized to real proportions. But coordinates we are using for prediction are not scaled accordingly and the actual patch we are making prediction on differs from the expected patch. For example, the zooming factor for full LIDC-IDRI-0003 image is [2.5, 0.820312, 0.820312]. The prediction result for **coordinates of the real nodule** {'x': 367, 'y': 350, 'z': 72} is **0.0066**. As you can see, it's very low. But if you scale these coordinates by the current zooming factor ({'x': 301, 'y': 286, 'z': 180}) we will get much higher probability of concerning **0.42**. More details [here](https://github.com/concept-to-clinic/concept-to-clinic/issues/268#issuecomment-354797905). ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
10 months, 1 week ago

@Serhiy-Shekhovtsov commented on issue #268: Classification throws RuntimeError for real nodule location

> Is there instead, another coordinate that works for both large and small images? Yes, there is. Don't have it at my hand right now, but will post within an hour.
10 months, 1 week ago

@Serhiy-Shekhovtsov commented on issue #131: Continuous improvement of nodule classification models (see #2)

@reubano, @pjbull what @WGierke [said here](https://github.com/concept-to-clinic/concept-to-clinic/issues/131#issuecomment-351563298) makes perfect sense to me. If the score for those metrics is a required part of models improvement it would be nice to have a set of standard tests for them. I am happy to take part in it's development. What do you think?
10 months, 2 weeks ago
10 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on issue #268: Classification throws RuntimeError for real nodule location

Probably, zooming factor from [this place](https://github.com/concept-to-clinic/concept-to-clinic/blob/666fb1bf410cbd80eb7928d0e3c5f2433ad668a5/prediction/src/preprocess/preprocess_ct.py#L141) should be saved as a property of CT [MetaData](https://github.com/concept-to-clinic/concept-to-clinic/blob/666fb1bf410cbd80eb7928d0e3c5f2433ad668a5/prediction/src/preprocess/load_ct.py) object and then used by [crop_patch](https://github.com/concept-to-clinic/concept-to-clinic/blob/666fb1bf410cbd80eb7928d0e3c5f2433ad668a5/prediction/src/preprocess/crop_patches.py) method instead of spacing? @WGierke, @reubano
10 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on issue #268: Classification throws RuntimeError for real nodule location

The mentioned issue is very simple - test is running on small DICOM image and 72 is far outside the boundaries of Z dimension(0, 28). The cropping will fail to create a patch of expected size (96, 96, 96). The test can be fixed by simple replacement of `dicom_path_003`(small image) with `dicom_paths[2]`(full image). While troubleshooting this issue I have found an other problem - the preprocessing is [zooming the image](https://github.com/concept-to-clinic/concept-to-clinic/blob/666fb1bf410cbd80eb7928d0e3c5f2433ad668a5/prediction/src/preprocess/preprocess_ct.py#L144). But coordinates we are using for prediction are not scaled according to the zoom and the actual patch we are making prediction on differs from the expected patch. For example, the zooming factor for full LIDC-IDRI-0003 image is [2.5, 0.820312, 0.820312]. The prediction result for **coordinates of the real nodule** {'x': 367, 'y': 350, 'z': 72} is **0.0066**. As you can see, it's very low. But if scale these coordinates by the current zooming factor ({'x': 301, 'y': 286, 'z': 180}) we will get much higher probability of concerning **0.42**. I am looking into it.
10 months, 2 weeks ago
10 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on issue #268: Classification throws RuntimeError for real nodule location

I did some checking... I may be wrong, but it looks like the specified slice is not valid. The one we are running the test for is `{'x': 367, 'y': 349, 'z': 72}` but the DICOM at this slice looks like a mess: ![nodule1](https://image.prntscr.com/image/BSyb534nQ1CKV30UU8hdsg.png) While when I run the test on `{'x': 367, 'y': 349, 'z': 189}` it doesn't fail. Here is how the image looks at specified slice: ![nodule1](https://image.prntscr.com/image/b-VYmjN2Tra4JL_fAZpj9g.png) @WGierke, @reubano what do you think?
10 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on PR #270: DICOM viewer: zoom and pan support

Looks like the build has been failing since [this PR](https://travis-ci.org/concept-to-clinic/concept-to-clinic/builds/322566530).
10 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on issue #131: Continuous improvement of nodule classification models (see #2)

@WGierke, I have create a virtual machine, the support should increase the instances limit anytime soon, and then the machine will be ready for use. Also, I have plenty of time for upcoming week. So, if you have some ideas, we can work together on the issue and share the points and the fun :) If this sounds good to you, please, [contact me](https://gitter.im/Serhiy-Shekhovtsov) on Gitter.
10 months, 2 weeks ago

@Serhiy-Shekhovtsov opened a new pull request: #270: DICOM viewer: zoom and pan support

Enabled window width and window center adjustment. Enabled zooming and panning. Updated the `NoduleMarker` component to rescale and move the marker accordingly. Fixed bug - moving slider to cached slices didn't show those slices. ## Reference to official issue #240 ## Screenshot ![zoom, pan](http://g.recordit.co/WF64msW6C0.gif) ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
10 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on issue #240: DICOM viewer: zoom, pan, and navigate through slices

Activated window width and window center adjustment, zooming and panning of cornerstone tools component. Updated the `NoduleMarker` component to rescale and move the marker accordingly. ![zoom, pan](http://g.recordit.co/VbxLfPX3qB.gif) Will push as soon as PR #269 is merged.
10 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on issue #240: DICOM viewer: zoom, pan, and navigate through slices

Looks like the new cornerstone version have support for zoom and pan. Will check how can we use it with our components for nodule selection and segmentation.
10 months, 3 weeks ago

@Serhiy-Shekhovtsov opened a new pull request: #269: Updated cornerstone-tools, removed jquery-slim dependency

## Description Fixed the code to work with new version of `cornerstone-tools`, updated references in `package.json`. ## Reference to official issue #215 ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
10 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on PR #260: fixed candidate select\deselect API test

@lamby maybe I am missing something, what test do you mean?
11 months ago

@Serhiy-Shekhovtsov commented on PR #260: fixed candidate select\deselect API test

@reubano, @lamby I have solved conflicts and solved one issue with my test. I can see that build is still failing but can't see how this is related to my changes. All failing tests are failing by timeout and called from `test_endpoints`.
11 months, 1 week ago

@Serhiy-Shekhovtsov opened a new pull request: #260: fixed candidate select\deselect API test

Small fix of the API tests based on this [discussion](https://github.com/concept-to-clinic/concept-to-clinic/pull/254#issuecomment-348448307). ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
11 months, 1 week ago

@Serhiy-Shekhovtsov commented on PR #254: Detect and select: mark\dismiss candidates

Guys, test are a little bit different. Old tests were checking the API output while new one is testing outcome(the DB result).
11 months, 1 week ago

@Serhiy-Shekhovtsov commented on PR #233: #145 import image series

_We should not show the list of files under last folder. Because once we switch to real data - we'll see **hundreds of files** there. It will make it complicated for user to browse the tree and this list doesn't do any help, cause the DICOM viewer should already support scrolling through slices._ This is still not fixed, right?
11 months, 1 week ago

@Serhiy-Shekhovtsov commented on PR #254: Detect and select: mark\dismiss candidates

I still have to add API test for this feature. Will make a new PR for that.
11 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on PR #233: #145 import image series

Great job @vessemer! Few small comments from my side: - there are two buttons on the page: **Create case** and **Start case** and one of them does nothing but confuses the user :) - also I think we should not show the list of files under last folder. Because once we switch to real data - we'll see **hundreds of files** there. It will make it complicated for user to browse the tree and this list doesn't do any help, cause the DICOM viewer should already support scrolling through slices. Again, you can copy\merge my [code here](https://github.com/concept-to-clinic/concept-to-clinic/pull/236/files#diff-c93691d2fd82606ab2bb19978545678fL2).
11 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on PR #233: #145 import image series

@lamby, @reubano, I can reopen and update my PR if it will help us to proceed on this.
11 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on PR #233: #145 import image series

I can reopen and update my PR if it will help us to proceed on this.
11 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on PR #250: Detect and select: attaching nodule centroid marker to current slice

That's because dummy candidates are located on random and often missing slices. Opening a candidate will try to open that slice.
11 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on PR #250: Detect and select: attaching nodule centroid marker to current slice

@reubano this will give you the data: ``` from backend.cases.factories import * from backend.cases.models import * from backend.images.models import * # drop existing case(s) with candidates and nodules Case.objects.all().delete() # drop imported images ImageSeries.objects.all().delete() # import a new image and start a new case new_image, created = ImageSeries.get_or_create('/images/LIDC-IDRI-0003/1.3.6.1.4.1.14519.5.2.1.6279.6001.101370605276577556143013894866/1.3.6.1.4.1.14519.5.2.1.6279.6001.170706757615202213033480003264') new_case = CaseFactory(series=new_image) # dummy data candidates = CandidateFactory.create_batch(5, case=new_case) NoduleFactory(candidate=candidates[0], case=new_case) NoduleFactory(candidate=candidates[1], case=new_case) NoduleFactory(candidate=candidates[4], case=new_case) ```
11 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on PR #250: Detect and select: attaching nodule centroid marker to current slice

I had the **Start case** feature implemented and working on my end. So I had the data with proper `series.uri` values. But we had conflicting Pull Requests for the same feature and I closed [mine one](https://github.com/concept-to-clinic/concept-to-clinic/pull/236) if favor of [an other](https://github.com/concept-to-clinic/concept-to-clinic/pull/233).
11 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on PR #250: Detect and select: attaching nodule centroid marker to current slice

This is because of bad dummy data. Unfortunately #145 is still not closed, so the proper way to start the case doesn't work.
11 months, 2 weeks ago

@Serhiy-Shekhovtsov opened a new pull request: #254: Detect and select: mark\dismiss implementation

Implemented functionality for reviewing nodule candidates. Now user can set whether the nodule was marked for further analysis or not. **NOTE:** this PR depends on #250 and has only 3 commits on top of it. ## Reference to official issue It's an improvement of #148 ## Screenshot: ![detect-and-select-review](http://g.recordit.co/MlvpaNE9cB.gif) [full size image](http://g.recordit.co/MlvpaNE9cB.gif) ## CLA - [x] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
11 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on PR #250: Detect and select: attaching nodule centroid marker to current slice

@reubano, @lamby PR has been fixed by rebasing it of top of [last commit](https://github.com/concept-to-clinic/concept-to-clinic/commit/e948d7f4733d7c215c76c96ba9505f3045a1c4ba) that actually fixed the issue. > but clicking 'Detect and Select' doesn't do anything. Maybe it's because you don't have any data on your environment? You can generate some fake data with this code: ``` from backend.cases.factories import * case = CaseFactory() candidates = CandidateFactory.create_batch(5, case=case) nodule0 = NoduleFactory(candidate=candidates[0]) nodule1 = NoduleFactory(candidate=candidates[1]) nodule4 = NoduleFactory(candidate=candidates[4]) ``` The process will be much easier as soon as we close #145
11 months, 2 weeks ago

@Serhiy-Shekhovtsov commented on issue #150: Vue: implement 'Segmentation' UI component(s)

@lamby I think it's too early to close this one yet :) I've only implemented a component for area selection, but still there things to be implemented: - Display an image view of a given slice of the selected nodule. Allow navigation between slices. - Code from #148 can be used as an example - Overlay the current vertices on top of the nodule image to see the predicted boundary. - The component is ready, we should read the area and initialize `AreaSelect` - Allow the user to move existing vertices. ✔ already supported by `AreaSelect`, we should listen to `selection-changed` event and update the model - Allow the user to delete existing vertices. - Allow the user to add a new vertex splitting a line segment connecting two vertices. - Not sure what this is means :) - Allow the user to extend the nodule boundary along the Z axis by extending the currently terminal polygon up or down to the adjacent slice which still has unhighlighted nodule. - We should observe `stack.currentImageIdIndex` change and reinit `AreaSelect` to display the nodule area at current slice - Indicate to the user the Z axis extent of slices. ✔ already supported by DICOM viewer
11 months, 3 weeks ago

@Serhiy-Shekhovtsov opened a new pull request: #250: Detect and select: attaching nodule centroid marker to current slice

Implemented slice switching for candidate list, and a way for user to see when slice differs and attach the marker to current slice. ## Description Following features has been implemented: - switching the viewer to candidate's slice when selecting a candidate - showing to user that marker is on an other slice - clicking on a marker attaches it to current slice On back-end I have added a method for loading candidates with cases and series. Series object will also have list of DICOM files in the folder saved in `uri` property. ## Reference to official issue It's an improvement of #148 ## Screenshot: ![detect-and-select-z](http://g.recordit.co/6AwCrut5WW.gif) [full size image](http://g.recordit.co/6AwCrut5WW.gif) ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
11 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on issue #150: Vue: implement 'Segmentation' UI component(s)

@vessemer I have copied and improved the [mentioned project](https://github.com/neshte/jquery-canvas-area-draw) to fit our needs in this PR: #249. ![vue-area-select](http://g.recordit.co/HERuf4ckeD.gif)
11 months, 3 weeks ago

@Serhiy-Shekhovtsov opened a new pull request: #249: implemented area select vue component for Segmentation page (#150)

Implemented Vue.js component for area selection based on [this project](https://github.com/neshte/jquery-canvas-area-draw). Did following updates: - converted to Vue component - get rid of jQuery dependency - added ability to move entire area when `Shift` is pressed ![vue-area-select](http://g.recordit.co/HERuf4ckeD.gif) Can be used as simple as this: `<area-select @selection-changed="areaSelectChange" v-if="showAreaSelect" :selectedArea="currentArea"></area-select>` ## Reference to official issue #150 ## License Since I used [this project](https://github.com/neshte/jquery-canvas-area-draw) as a starting point, should I include it's license in the code? ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
11 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on PR #233: #145 import image series

One more thing, since you have fixed the DICOM viewer - user can scroll through slices now. So, clicking on last folder should actually show the image. No need to show list of files. You can merge my [code here](https://github.com/concept-to-clinic/concept-to-clinic/pull/236/files#diff-c93691d2fd82606ab2bb19978545678fL2).
11 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on PR #236: Implemented Imagery selection screen

Guys, I am closing this PR in favor of #233. Will post my thoughts there.
11 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on PR #236: Implemented Imagery selection screen

Switching between cases is good idea. From my point of view, the following changes could improve the #233: ## No need to show Import button and require user to click it. **Show the list of cases** and **the TreeView with files by default**. User should be able to switch the case or start a new case with minimum amount of clicks. ![open image](https://image.prntscr.com/image/E7lKMrBuRd_ssnWxwcu5Mw.png) ## No need to require user to import image before starting a new case. Nor to show the redundant information. On the backend you can use `ImageSeries.get_or_create(uri)` method to load existing ImageSeries for a new Case. ## Accordion like behavior for files TreeView Because user should see what file he is previewing now. In current version user can unfold multiple folders same time and it's not clear what DICOM is opened in the viewer at the moment. This is the reason I used `EmitBus`. If you can see a better way to implement it - I have nothing against it. If these changes are OK with you guys, @vessemer can update his PR and I will close my in favor of it. Alternatively, I can merge the list of cases from #233 to my PR.
11 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on issue #150: Vue: implement 'Segmentation' UI component(s)

Have a look at [Area Canvas Editor](https://github.com/neshte/jquery-canvas-area-draw) as well: ![Area Canvas Editor](http://g.recordit.co/dTjU1H0KV0.gif) It has some advantages over ROI tool: 1. no `cornerstone-tools` dependency, you can put it in any div element. and you can make a separate `Vue.js` component 2. user can add new points to existing area 3. user can remove points 4. dragging of entire area. It doesn't work on demo page, but I can see it in the code, so we can debug and fix it 5. the code looks pretty clear, we can probably improve it and get rid of jQuery dependency I didn't start implementing such component yet, only did small research.
11 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on PR #226: Implement 'Detect and Select' component (#148)

@reubano I can't reproduce the issue you mentioned. Also the Travis build passed. Can you check it again, please?
11 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on issue #150: Vue: implement 'Segmentation' UI component(s)

I am taking it. If anyone willing to join - please, put a comment or ping me on [gitter](https://gitter.im/concept-to-clinic/Lobby), so we can collaborate on this.
11 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on PR #236: Implemented Imagery selection screen

@lamby my PR fits with requirements and easier for user: you can see images on the local system by default, click Preview and click Start new case. While PR #233 requires the user to click Import first, click Preview image, then he has to add an image(by clicking Import) and then Create a case. From my point of view, those steps are redundant and don't match with [requirements](http://concept-to-clinic.readthedocs.io/en/latest/design-doc.html#imagery-interface-api). Of course, @vessemer can still update his PR, but I can't see much sense in this. If points is an issue - I have no problem with sharing them between us.
11 months, 3 weeks ago
11 months, 3 weeks ago
11 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on issue #150: Vue: implement 'Segmentation' UI component(s)

@jjjmm are you still working on this issue? If yes, do you need any help?
11 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on issue #241: DICOM viewer: errors in console and can't select slice

I have created a task for all features the **DICOM viewer** should have - #240 and probably it will take some time to implement them. Till that time this bug can fixed separately.
11 months, 3 weeks ago

@Serhiy-Shekhovtsov created a new issue: #241: DICOM viewer - errors in console and can't select slice

When importing\browsing DICOM images the viewer is showing only once slice and doesn't allow scrolling. I can see following errors in console: > [Vue warn]: Error in callback for watcher "view.paths": "Error: updateImage: image has not been loaded yet" > > found in > > ---> <OpenDicom> at src/components/open-image/OpenDICOM.vue > <ImageSeries> at src/components/open-image/ImageSeries.vue > <OpenImage> at src/views/OpenImage.vue > <App> at src/App.vue > <Root> And this one: > updateImage: image has not been loaded yet > at Object.exports.default (cornerstone.js?f051:309) > at Object.enable (cornerstoneTools.js?a9d4:1797) > at VueComponent.initCS (OpenDICOM.vue?67e0:112) > at VueComponent.boundFn [as initCS] (vue.esm.js?65d7:188) > at VueComponent.viewPaths (OpenDICOM.vue?67e0:49) > at Watcher.run (vue.esm.js?65d7:3175) > at flushSchedulerQueue (vue.esm.js?65d7:2927) > at Array.eval (vue.esm.js?65d7:1792) > at MessagePort.flushCallbacks (vue.esm.js?65d7:1713) ## Steps to Reproduce 1. Open home page 2. Click Import 3. Unfold until you find an image file 4. Click on image file ## Checklist before submitting - [ ] I have confirmed this using the officially supported Docker Compose setup using the `local.yml` configuration and ensured that I built the containers again and they reflect the most recent version of the project at the `HEAD` commit on the `master` branch - [ ] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [ ] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [ ] I provided **screenshots** where appropriate - [ ] I filled out all the relevant sections of this template
11 months, 3 weeks ago

@Serhiy-Shekhovtsov created a new issue: #240: DICOM viewer: zoom, pan, and navigate through slices in the image viewer

According to documentation: > Allow the user to zoom, pan, and navigate through slices in the image viewer. During these actions the following model properties should be updated: `zoomRate`, `offsetX`, `offsetY`. These properties has been introduced in [this commit](https://github.com/concept-to-clinic/concept-to-clinic/pull/226/commits/9e53048dbd65d8d232787e957ba2db2b7f622c48#diff-70d179422138a3042c2135a282065626R42). Hopefully PR will be merged soon. These properties should taken into by "Detect and Select" and "Segmentation" components (#226 and #150) ## Current Behavior User can't navigate through slices, nor zoom+pan. ## Checklist before submitting - [ ] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [ ] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [ ] I provided **screenshots** where appropriate - [ ] I filled out all the relevant sections of this template
11 months, 3 weeks ago
11 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on issue #145: 'Import' UI element should be able to create backend `ImageSeries` model instance

We have two pull requests for this issue: PR #233 with this approach: 1. Click Import 2. Preview images on local system 3. Click Load Image 4. Start new case And PR #236 with the approach: 1. Preview images on local system 2. Start new case And there some changes requested by @louisgv that are applicable for both of these pull requests. But I am not sure about working on them, since @vessemer may be implementing the same thing. My [comment asking if anyone working on the issue](https://github.com/concept-to-clinic/concept-to-clinic/issues/145#issuecomment-344731667), as well as my concern about collaboration over competition [here](https://github.com/concept-to-clinic/concept-to-clinic/pull/233#issuecomment-345512818) was silently ignored by @vessemer. @lamby would you mind stepping in here and share your thoughts? To be clear - I am not concerned about awarded points, but about transparency and fairness. We have plenty of other tasks to do here, so what's the point in wasting our time working on same things?
11 months, 3 weeks ago

@Serhiy-Shekhovtsov commented on PR #233: #145 import image series

Image loading is also a part of [this PR](https://github.com/concept-to-clinic/concept-to-clinic/pull/236) that also shows currently running case, allows user to import and start new case after preview image, as it was described in [documentation](http://concept-to-clinic.readthedocs.io/en/latest/design-doc.html#imagery-interface-frontend).
11 months, 4 weeks ago

@Serhiy-Shekhovtsov commented on PR #226: Implement 'Detect and Select' component (#148)

@lamby, @reubano is there anything blocking the merge of this PR?
11 months, 4 weeks ago

@Serhiy-Shekhovtsov commented on PR #233: #145 import image series

@vessemer, I think it's a good idea to let others know when you are working on an issue (like I did [here](https://github.com/concept-to-clinic/concept-to-clinic/issues/145#issuecomment-344731667)) if we are choosing collaboration over competition. This way team members will not waste their time working on same tasks.
11 months, 4 weeks ago

@Serhiy-Shekhovtsov opened a new pull request: #236: Implemented Imagery selection screen

## Description According to [documentation](http://concept-to-clinic.readthedocs.io/en/latest/design-doc.html#imagery-interface-frontend) on this screen should in particular: - List available DICOM images on local filesystem. - Display a preview of an available DICOM image. - Allow user to select a given image and **start a new “Case.”** This image will be used in the identification step next. All these features has been implemented in this PR(frontend and backend). - Now user can see details about currently running case - Can see list of directories and preview an image - Can start a new case for selected image ![start a new case](http://g.recordit.co/Htja5hXlvc.gif) ## Reference to official issue Related to #145 and #202 ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
11 months, 4 weeks ago

@Serhiy-Shekhovtsov commented on issue #202: Improve open imagery "Import" button UX

!["Import" button UX](http://g.recordit.co/QvlvS3OCv6.gif)
11 months, 4 weeks ago

@Serhiy-Shekhovtsov opened a new pull request: #235: fixed Vue.js compilation error by updating Vue.js version

Because of [this bug](https://github.com/vuejs/vue/issues/7084) in version 2.4.2 of **Vue.js** the compilation of Vue app was broken. I have fixed it by updating the package. ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
11 months, 4 weeks ago

@Serhiy-Shekhovtsov commented on PR #232: fixed Vue.js startup issue

@lamby after checking this again I have found that it's a Vue.js bug and it has been fixed in latest version. Will fix my PR. You can check more details [here](https://github.com/vuejs/vue/issues/7084), but basically it was throwing an error when trying to pass `v-model="..."` and `:value="..."` same time to one element.
11 months, 4 weeks ago
11 months, 4 weeks ago

@Serhiy-Shekhovtsov commented on issue #202: Improve open imagery "Import" button UX

@louisgv , but I already started #202. And this one looks like really minor and will be closed as well.
11 months, 4 weeks ago

@Serhiy-Shekhovtsov commented on issue #202: Improve open imagery "Import" button UX

I am going to do the following: - Show if there is any case started. If started - show a message that starting a new case will drop the current one. - Show list of images in local system. User can click on image name to view it. (this is already implemented) - On "start case" button click - I will drop the current case and create a new one with selected image.
11 months, 4 weeks ago

@Serhiy-Shekhovtsov commented on issue #202: Improve open imagery "Import" button UX

I am not sure if we should allow selecting multiple images. In documentation we can find the following: > Allow user to select a given image and start a new “Case.” This image will be used in the identification step next Also it will complicate some things, including this story: Prevent progress to next stage of analysis until "completed" the current step (#204). Please, let me know what do you think? (This issue is blocking me with #145)
11 months, 4 weeks ago

@Serhiy-Shekhovtsov opened a new pull request: #232: fixed Vue.js startup issue

Front-end application was not working because of the following error: `:value conflicts with v-model on the same element because the latter already expands to a value binding internally` ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
11 months, 4 weeks ago

@Serhiy-Shekhovtsov commented on issue #228: Any reason for having HyperlinkedModelSerializer instead of ModelSerializer?

@isms, your suggestion to inherit all from base and override the `get_serializer_context` is nice, didn't think about it. And after I have implemented it the issue with `.json` is also gone. Btw, here are my findings about `.json` issue: When request is *not* set to None for the serializer, then generated URL will be absolute **and also** copy the extension of the query. If you load `/api/candidates.json` each candidate will have URL `xxx/api/candidates/1.json`.
11 months, 4 weeks ago

@Serhiy-Shekhovtsov opened a new pull request: #231: Fixed interface API to return relative urls instead of absoulte

Now each `[Model]ViewSet` will inherit from `ViewSetBase` and that base view class has `get_serializer_context` method overridden to set `'request': None` for the serialization context. ## Description After this change serialized entities have relative URLs instead of absolute and it makes them usable from the Vue.js application. Problem with absolute URLs - interface API is hosted on different port number then *Vue.js* and requests to that port number being blocked by browser because of CORS policy. Now the URLs are pointing to the same host and will be redirected by *Node.js* server. Fix suggested @isms when discussing #228 ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
11 months, 4 weeks ago

@Serhiy-Shekhovtsov created a new issue: #228: Any reason for having HyperlinkedModelSerializer instead of ModelSerializer?

Is there any reason for using `HyperlinkedModelSerializer` for serializing the data before sending it to client side? ### Having this serializer have following drawbacks: - Returned data doesn't have the **id**. Here is an example of data for `/api/candidates.json` method: `{"url":"http://interface:8001/api/candidates/1.json","centroid":{ ... },"created":" ... ","probability_concerning":0.3,"case":"http://interface:8001/api/cases/1.json"}` - Returned URL is not usable. - As you can see, it's absolute and since the port doesn't match the Vue.js server's port - AJAX requests to that URL are blocked by the browser. - Also the url contains **.json** extension. For example, services we have for marking/dismissing candidates look like this: `candidates/1/mark`, `candidates/1/dismiss` ### So, what are the benefits? Maybe I am missing something here, since I am totally new to django, but it looks like replacing `HyperlinkedModelSerializer` with `ModelSerializer` helps to solve the issue of missing **id** and it also doesn't return unusable urls. ## Possible Solution AFAIK, we can still use `HyperlinkedModelSerializer` but patch it by overriding a method for creating serializer and passing `None` as `request`. Or we can simply use to `ModelSerializer`.
12 months ago

@Serhiy-Shekhovtsov commented on issue #215: Eliminate/Restrict Jquery use for cornerstone-tools

@truefalse10 btw, looks like they have a branch for this: https://github.com/chafey/cornerstone/tree/zachasme-es6 More details [here](https://github.com/chafey/cornerstone/issues/180) and [here](https://github.com/chafey/cornerstone/issues/92)
12 months ago

@Serhiy-Shekhovtsov opened a new pull request: #226: Implement 'Detect and Select' component (#148)

Implemented draggable marker for Detect and Select page for issue #148 ## Description [vue-draggable-resizable](https://github.com/mauricius/vue-draggable-resizable) component used to implement draggable nodule marker. The marker is implemented as a separate component (open-image\NoduleMarker.vue) it takes `marker` (object with coordinates) param and navigation\zoom params `zoomRate`, `offsetX`, `offsetY` ## Screenshots (if appropriate): ![Detect and Select](http://g.recordit.co/wtf2QJwtYP.gif) ### What is not done: - loading image from backend(it requires a method for loading currently selected case) - saving mark\dismiss\coordinates on backend If someone familiar with backend side would like to jump in and help to complete this page - I am more then happy to help from client side. ## CLA - [x] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
12 months ago

@Serhiy-Shekhovtsov commented on PR #197: #148 Manipulations with DICOM vue

Guys @lamby, @louisgv, these changes are very important for #148 and #150. In fact, I can't proceed with #148 without it. Can we merge the current request and create a new issue for "slimmer version of jquery"? P.S. great job, @vessemer!
1 year ago

@Serhiy-Shekhovtsov commented on PR #213: Fixed nodule identification

@lamby, thanks for your feedback I will check how to get rid of those merge commits and fix comments(by comments you mean commit messages, right?).
1 year ago

@Serhiy-Shekhovtsov opened a new pull request: #213: Fixed nodule identification

Fixed #191 (Nodule identification doesn't work) ## Description The problem was - passing image object to `gtr123_model.predict` method, when this method expects a path to a DICOM image. ## How Has This Been Tested? I checked that it starts with right input, but can't do the full run because of *not enough memory* error. ## CLA - [x] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
1 year ago

@Serhiy-Shekhovtsov opened a new pull request: #212: Fixed nodule identification

Fixed #191 (Nodule identification doesn't work) ## Description The problem was - passing image object to `gtr123_model.predict` method, when this method expects a path to a DICOM image. ## How Has This Been Tested? I checked that it starts with right input, but can't do the full run because of *not enough memory* error. ## CLA - [ x ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
1 year ago

@Serhiy-Shekhovtsov commented on issue #205: Export PDF functionality

@lamby, I can see the **interface-backend** tag, is there a particular reason why this has to be done on the server side? I am asking this, cause implementing it on the client side should be an easy task.
1 year ago

@Serhiy-Shekhovtsov created a new issue: #145: Import image series

<!--- Provide a general summary of the issue in the Title above --> I can't find how to import images for opening. Is this feature implemented? If it's not - is there a workaround for importing? ## Expected Behavior User should be able to import images displayed in "Import image series" section. ## Current Behavior There is tree view for browsing file system, but there is no way to select and import file\folder. ![image](https://user-images.githubusercontent.com/607527/31047402-c4037f9e-a612-11e7-8f24-c3ae82b867d5.png) ## Steps to Reproduce Browse http://0.0.0.0:8080 and click **Import** button. ## Checklist before submitting - [x] I have confirmed this using the officially supported Docker Compose setup using the `local.yml` configuration and ensured that I built the containers again and they reflect the most recent version of the project at the `HEAD` commit on the `master` branch - [x] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [x] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [x] I provided **screenshots** where appropriate - [x] I filled out all the relevant sections of this template
1 year, 1 month ago

@Serhiy-Shekhovtsov created a new issue: #144: Slack workspace

Guys, what do you think about creating a slack workspace for team communication? Many times when I got into a problem\question I don't know where to put it (forum is not active). Also there is no simple\proper way to reach the person, if question is not related to specific issue or pull request.
1 year, 1 month ago

@Serhiy-Shekhovtsov commented on issue #4: Feature: Adapt the grt123 model

@dchansen I am getting this error when trying to checkout your repo: > $ git-lfs.exe smudge -- prediction/src/algorithms/classify/assets/gtr123_model.ckpt > Error downloading object: prediction/src/algorithms/classify/assets/gtr123_model.ckpt (eb2c037dd55e2f49da95657f7a7851cbcfe2f2b516848ed03f8c5c820f3e16b4) > > Smudge error: Error downloading prediction/src/algorithms/classify/assets/gtr123_model.ckpt (eb2c037dd55e2f49da95657f7a7851cbcfe2f2b516848ed03f8c5c820f3e16b4): [eb2c037dd55e2f49da95657f7a7851cbcfe2f2b516848ed03f8c5c820f3e16b4] Object does not exist on the server: [404] Object does not exist on the server Any ideas what is it and how to solve it?
1 year, 1 month ago

@Serhiy-Shekhovtsov commented on issue #4: Feature: Adapt the grt123 model

> Do you get the same error when running the base repo? Yes. > I have the full model adapted for Python3 in the pull request at #122 Great!
1 year, 1 month ago

@Serhiy-Shekhovtsov commented on issue #1: Feature: Implement identification algorithm

Hi @dchansen, I am working on a related issue: adapting grt123 model(#4). You said you already did that, where can I check your code? Also, if the adaptation is done, I guess we can close the #4.
1 year, 1 month ago

@Serhiy-Shekhovtsov commented on issue #4: Feature: Adapt the grt123 model

Hi @reubano! I've fixed few issues on pre-processing part and managed to have it running successfully and giving the same result as original code. These changes have been pushed now. Now I am looking into detection part, but encountered this issue: > Torch: not enough memory: you tried to allocate 21GB Does it really take this much memory or is it an issue with environment \ code?
1 year, 1 month ago

@Serhiy-Shekhovtsov commented on issue #4: Feature: Adapt the grt123 model

I'd like to work on this one. P.S. btw, is it a good idea to tell when you are working on something?
1 year, 2 months ago

@Serhiy-Shekhovtsov opened a new pull request: #95: issue #91: moving small DICOM files from lfs to git

Moving small files from lfs to git ## Description I checked these examples: [1](https://stackoverflow.com/a/41961459/246719) and [2](https://grep.be/blog//en/computer/Removing_git-lfs/) And moved files by following these steps: 1. Modified `.gitattributes` by adding this line for untracking files under `test_image_data/small`: `tests/assets/test_image_data/small/**/*.dcm -filter=lfs -diff=lfs -merge=lfs -text` 2. Marked files as removed from git `git rm --cached tests/assets/test_image_data/small/**/*.dcm` 3. Added files to the index as normal files `git add tests/assets/test_image_data/small/**/*.dcm` ## How Has This Been Tested? Created a new empty folder and checked out the repository using following commands: `GIT_LFS_SKIP_SMUDGE=1` `git clone https://github.com/Serhiy-Shekhovtsov/concept-to-clinic.git` After the checkout I can see that all full files are downloaded as references(each file has 131 bytes) but all images under `tests/assets/test_image_data/small` folder have been downloaded as normal files. ## Reference to official issue Issue #91 ## CLA - [x] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
1 year, 2 months ago

@Serhiy-Shekhovtsov commented on issue #89: Can't get the system running

Ok, issue solved now. The site can't be reached with `localhost` or `container_ip` but can be reached using `DOCKER_HOST ` variable. Command `docker-machine env` can be used to get this value.
1 year, 2 months ago

@Serhiy-Shekhovtsov commented on issue #89: Can't get the system running

The most recent log looks good: > Starting repo_prediction_1 ... > Starting repo_postgres_1 ... > Starting repo_prediction_1 > Starting repo_prediction_1 ... done > Starting repo_interface_1 ... > Starting repo_interface_1 ... done > Attaching to repo_postgres_1, repo_prediction_1, repo_interface_1 > postgres_1 | LOG: database system was shut down at 2017-09-04 20:30:58 UTC > postgres_1 | LOG: MultiXact member wraparound protections are now enabled > interface_1 | Postgres is up - continuing... > interface_1 | Hello World > interface_1 | ['assets', 'backend', 'config', 'frontend', 'manage.py', 'requir > ements', 'requirements.txt', '__pycache__'] > prediction_1 | * Serving Flask app "src.factory" > prediction_1 | * Forcing debug mode on > prediction_1 | * Running on http://0.0.0.0:8001/ (Press CTRL+C to quit) > interface_1 | /usr/local/lib/python3.6/site-packages/environ/environ.py:618: U > serWarning: Error reading /app/.env - if you're not configuring your environment > separately, check this. > interface_1 | "environment separately, check this." % env_file) > interface_1 | Operations to perform: > interface_1 | Apply all migrations: auth, cases, contenttypes, images, sessi > ons > interface_1 | Running migrations: > prediction_1 | * Restarting with inotify reloader > prediction_1 | * Debugger is active! > interface_1 | No migrations to apply. > interface_1 | /usr/local/lib/python3.6/site-packages/environ/environ.py:618: U > serWarning: Error reading /app/.env - if you're not configuring your environment > separately, check this. > interface_1 | "environment separately, check this." % env_file) > interface_1 | * Running on http://0.0.0.0:8000/ (Press CTRL+C to quit) > interface_1 | * Restarting with stat > interface_1 | /usr/local/lib/python3.6/site-packages/environ/environ.py:618: U > serWarning: Error reading /app/.env - if you're not configuring your environment > separately, check this. > interface_1 | "environment separately, check this." % env_file) > interface_1 | Performing system checks... > interface_1 | > interface_1 | System check identified no issues (0 silenced). > interface_1 | > interface_1 | Django version 1.11.3, using settings 'config.settings.local' > interface_1 | Development server is running at http://0.0.0.0:8000/ > interface_1 | Using the Werkzeug debugger (http://werkzeug.pocoo.org/) > interface_1 | Quit the server with CONTROL-C. > interface_1 | * Debugger is active! But still the interface project can't be reached from browser on host machine. I tried browsing `http://localhost:8000/` and `http://[container_ip]:8000/`
1 year, 2 months ago

@Serhiy-Shekhovtsov commented on issue #89: Can't get the system running

Yes, mounting the root repository folder fixed the sharing issue. The error above solved by removing and rebuilding the container.
1 year, 2 months ago

@Serhiy-Shekhovtsov commented on issue #89: Can't get the system running

@isms looks like it can be fixed with [this tool](https://github.com/vweevers/node-docker-share). After mounting `interface` folder it don't have problems locating `/app/manage.py`. Then I mounted the repo root folder and it's failing to mount the `HEAD`: > ERROR: for repo_interface_1 Cannot start service interface: oci runtime error: > container_linux.go:262: starting container process caused "process_linux.go:339: > container init caused \"rootfs_linux.go:57: mounting \\\"/d/Projects/DS/concept > -to-clinic/repo/.git/logs/HEAD\\\" to rootfs \\\"/mnt/sda1/var/lib/docker/aufs/m > nt/ebfe3deb31a87e6de1901640c13b0031e552ac7cb44244b1e80565a93ab062e5\\\" at \\\"/ > mnt/sda1/var/lib/docker/aufs/mnt/ebfe3deb31a87e6de1901640c13b0031e552ac7cb44244b > 1e80565a93ab062e5/HEAD\\\" caused \\\"not a directory\\\"\"" > : Are you trying to mount a directory onto a file (or vice-versa)? Check if the > specified host path exists and is the expected type Basically, point is `head` is not a directory, just like it says. I am looking into it.
1 year, 2 months ago

@Serhiy-Shekhovtsov commented on issue #89: Can't get the system running

@isms, docker maps the folder on host machine to a folder inside Docker container. When the **host** folder I am mapping is not inside home directory - the mapping doesn't work. So, when project is placed in `D:\Projects\DS\concept-to-clinic\repo` - it doesn't work. If I copy the project to `C:\Users\serhiy\concept-to-clinic` it's going to work. Does it make sense now?
1 year, 2 months ago

@Serhiy-Shekhovtsov commented on issue #89: Can't get the system running

The problem is - on windows we can't(?) mount the folder that is outside home directory (C:\users\username). More details [here](https://github.com/docker/compose/issues/2548). [This tool](https://github.com/vweevers/node-docker-share) looks promising.
1 year, 2 months ago

@Serhiy-Shekhovtsov commented on issue #89: Can't get the system running

And before that I also printed the cwd, it is `/app`.
1 year, 2 months ago

@Serhiy-Shekhovtsov commented on issue #89: Can't get the system running

He @reubano! `Docker version 17.07.0-ce, build 8784753` `docker-compose version 1.15.0, build e12f3b94` `docker-machine.exe version 0.12.2, build 9371605` Btw, I have put the following code to `start-dev.sh`: ``` echo "Hello World" python -c "exec(\"import os\\ncwd = os.getcwd()\\nprint(os.listdir(cwd))\")" python manage.py migrate python manage.py runserver_plus 0.0.0.0:8000 ``` and after rebuild and run it printed: interface_1 | Postgres is up - continuing... interface_1 | Hello World **interface_1 | []** interface_1 | python: can't open file 'manage.py': [Errno 2] No such file or directory
1 year, 2 months ago

@Serhiy-Shekhovtsov created a new issue: #89: Can't get the system running

After pulling the code and data I can't run the environment. The build was successful, but the command for taking projects up fails with following error: compile_docs_1 | make: Entering directory '/app/docs' compile_docs_1 | make: Leaving directory '/app/docs' compile_docs_1 | make: *** No rule to make target 'html'. Stop. postgres_1 | LOG: database system was shut down at 2017-09-04 12:24:18 UTC postgres_1 | LOG: MultiXact member wraparound protections are now enabled postgres_1 | LOG: database system is ready to accept connections interface_1 | Postgres is up - continuing... **interface_1 | python: can't open file 'manage.py': [Errno 2] No such file or directory repo_compile_docs_1 exited with code 2 interface_1 | python: can't open file 'manage.py': [Errno 2] No such file or directory repo_interface_1 exited with code 2** prediction_1 | * Serving Flask app "src/factory.py" prediction_1 | * Forcing debug mode on prediction_1 | * Running on http://0.0.0.0:8001/ (Press CTRL+C to quit) compile_docs_1 | make: *** /app/docs: No such file or directory. Stop. compile_docs_1 | make: Entering directory '/app/docs' compile_docs_1 | make: Leaving directory '/app/docs' compile_docs_1 | make: *** No rule to make target 'html'. Stop. And the _compile_docs_1 | make: ..._ messages set keeps repeating. ## Expected Behavior After running the `docker-compose -f local.yml up` command user should be able to open a browser and view the interface at http://localhost:8000 and the prediction API at http://localhost:8001 ## Current Behavior http://localhost:8000 nor http://localhost:8001 are not accessible. ## Steps to Reproduce 1. Take latest 2. run `docker-compose -f local.yml build` 3. run `docker-compose -f local.yml up` 4. browse http://localhost:8000 ## Environment Windows 7 x64, Docker Toolbox ## Checklist before submitting - [x] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug - [x] I provided a **minimal code snippet** or list of steps that reproduces the bug. - [x] I filled out all the relevant sections of this template
1 year, 2 months ago

@Serhiy-Shekhovtsov opened a new pull request: #87: Added a note about Docker Toolbox for Windows 7 users

The Docker will not run on Windows 7 but the error is not informative and someone(like me) could spend lots of time trying to find what is the issue and how to solve it. <!--- Provide a general summary of your changes in the Title above --> ## Description <!--- Describe your changes in detail --> ## Reference to official issue <!--- If fixing a bug, there should be an existing issue describing it with steps to reproduce --> <!--- Please link to the issue here: --> ## Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If adding a new feature or making improvements not already reflected in an official issue, please reference the relevant sections of the design doc --> ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## Screenshots (if appropriate): ## CLA - [ ] I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well
1 year, 2 months ago