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

Use position/direction sampling from base source for radionuclide sources #896

Merged
merged 1 commit into from
Jul 22, 2022

Conversation

ftessier
Copy link
Member

This pull request replaces #672. The original pull request branch was removed from the forked repository, so the branch and pull request is hereby reinstated. The original pull request message was:

The implementation of egs_radionuclude_source is changed so that it takes an input "base source" instead of defining a collimated or isotropic source. This way you can use any of the egs sources or a custom egs source to define the location of the radionuclide in the simulation. The change in particular was made towards fast simulations targeted radionuclide therapy across a whole patient phantom.

@ftessier ftessier requested a review from a team as a code owner July 11, 2022 16:17
@ftessier ftessier requested review from mainegra, rtownson and blakewalters and removed request for a team July 11, 2022 16:17
@ftessier ftessier force-pushed the feature-radionuclidesource-sourceimport branch from b75ca82 to e2c3448 Compare July 11, 2022 16:21
@ftessier
Copy link
Member Author

ftessier commented Jul 11, 2022

Rebased on develop.

@ftessier ftessier changed the base branch from master to develop July 11, 2022 16:27
@ftessier ftessier force-pushed the feature-radionuclidesource-sourceimport branch from e2c3448 to e715e5b Compare July 11, 2022 16:39
@ftessier
Copy link
Member Author

ftessier commented Jul 11, 2022

Something ain't right. The second (last) commit reverts apparently legitimate changes, for example adding the name of the contributor and updating the documentation. @MartinMartinov or @rtownson, any chance you can shed light on this? For the record, this pull request was reinstated from #672, from a branch obtained as follows:

git fetch origin pull/672/head:feature-radionuclidesource-sourceimport
git rebase develop
git push origin

@rtownson
Copy link
Collaborator

Yes the missing contributor/documentation looks like a mistake, not sure how it happened.

When you re-instate it, change the indentations to use spaces instead of tabs.

@ftessier ftessier added this to the Release 2022 milestone Jul 11, 2022
@ftessier ftessier self-assigned this Jul 11, 2022
@ftessier
Copy link
Member Author

Yes I will run astyle. So are you confirming that the other changes in that commit are all good then? I will attempt to clean it up and wait for review.

@rtownson
Copy link
Collaborator

Yes I will run astyle. So are you confirming that the other changes in that commit are all good then? I will attempt to clean it up and wait for review.

Yes, the comments from the first commit are correct. The code added in the second commit is also correct.

This PR breaks backwards compatibility - users with radionuclide sources will have to update their input files. I think that's OK, maintaining backwards compatibility would result in a lot of duplicate code.

@ftessier ftessier force-pushed the feature-radionuclidesource-sourceimport branch from e715e5b to 10892a1 Compare July 11, 2022 18:07
@ftessier
Copy link
Member Author

ftessier commented Jul 11, 2022

@rtownson I think I got it sorted out with the last push. Can you take a look? Applied astyle.

@rtownson
Copy link
Collaborator

@rtownson I think I got it sorted out with the last push. Can you take a look? Applied astyle.

Yep, looks right!

@rtownson
Copy link
Collaborator

rtownson commented Jul 12, 2022

I'm going to add one more commit to add a check for the old deprecated inputs, and give a description of the new format.

@ftessier ftessier force-pushed the feature-radionuclidesource-sourceimport branch from 0ebe771 to a3f7c5a Compare July 12, 2022 20:05
Change the egs_radionuclude_source to take an input "base source"
instead of defining a collimated or isotropic source. This way, any of
the egs++ sources or a custom source can now be used to define the
distribution of the radionuclide. Also, add warning messages for
deprecated radionuclide source inputs. This change was originally
motivated by simulations of targeted radionuclide therapy.
@ftessier ftessier force-pushed the feature-radionuclidesource-sourceimport branch from e655d6a to 7627126 Compare July 22, 2022 23:12
@ftessier
Copy link
Member Author

Rebased on develop, squashed and tweaked commit message. Also, remove the year from the contributor line in the header, to match EGSnrc style (only the author has the year indicated, to preserve pre-git history; now contribution date is provided by the git log)

@ftessier ftessier merged commit c986fd0 into develop Jul 22, 2022
@ftessier ftessier deleted the feature-radionuclidesource-sourceimport branch July 22, 2022 23:16
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.

5 participants