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

differential calculation modified #60

Merged
merged 2 commits into from
Aug 16, 2018
Merged

Conversation

inamoto85
Copy link
Contributor

Hi Bastula,
0 should be first appended at the end of the cumulative DVH and np.diff can then be used to differentiate the DVH.
In this way the information in last bin of the cumulative DVH can be preserved, comparing to append a 0 after the diff manipulation.

I have not understood the * -1. Would you explain a little bit?

And The modification would indeed fail related unittest (as you can see from the logfiles), But I believe this reasoning is correct.

Thank you for your time,

unittest.log

`0` should be first appended at the end of the cumulative DVH and `np.diff` can then be used to differentiate the DVH. 
In this way the information in last bin of the cumulative DVH can be preserved. I have not understood the `* -1`. Would you explain a little bit?
@bastula
Copy link
Member

bastula commented Aug 15, 2018

Hi Hideki,

I agree with this patch. I just did some testing and could verify your findings that the last cumulative count bin gets removed in the current implementation.

Regarding the * -1, I think it was a holdover from the original implementation from dicompyler and can be safely removed as there is an np.abs call.

I was actually hunting down this discrepancy some time ago but wasn't able to find it. Thanks!

I will merge in the changes with updated unit tests.

@bastula bastula merged commit 215b8fe into dicompyler:master Aug 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants