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

Add 3ddose_tools as a third-party submodule #906

Merged
merged 1 commit into from
Oct 31, 2022

Conversation

ftessier
Copy link
Member

@ftessier ftessier commented Jul 25, 2022

This pull request supersedes #632. The idea is to include the 3ddose_tools as a submodule in EGSnrc/third-party/, instead of including the source code itself within EGSnrc.

A submodule provides a way to include a git repository within another repository. This way, the submodule authors keep full control of their code development (and user support!) for the third-party software. In fact, EGSnrc is not distributing the software code at this point, but merely a reference to it. This means that developers can even distribute their code under different conditions, e.g., a different licence. This provides exposure as part of the EGSnrc project, and eventually the ability to synchronize the version of third-party content with each EGSnrc release.

When EGSnrc is cloned, submodule content is not downloaded by default (nor are they in the zip file of the EGSnrc project). Users can consult which submodules are available with the submodule command, for example:

$ git submodule
 ea2853a4223793827508ef3d6f64290b71599101 third-party/3ddose_tools (heads/master)

and then decide to download any listed submodule with the submodule update command:

$ cd third-party/3ddose_tools

$ git submodule init
Submodule 'third-party/3ddose_tools' (https://github.com/MartinMartinov/3ddose_tools) registered for path './'

$ git submodule update
Submodule path './': checked out 'ea2853a4223793827508ef3d6f64290b71599101'

If the submodule project advances to a new commit, the git submodule update command will pull the changes in the submodule directory.

Two global settings that are useful when working with submodules:

git config --global diff.submodule log
git config --global status.submodulesummary 1

I will add a README to give some information about third-party content and submodules.

Copy link
Collaborator

@rtownson rtownson left a comment

Choose a reason for hiding this comment

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

This is great! Let's do the same for egs_brachy and VICTORIA?

Edit: Maybe egs_brachy won't work, because it changes the EGSnrc directories..

@ftessier
Copy link
Member Author

ftessier commented Jul 26, 2022

Yes, I expect Victoria is easy to submodule.

I tried with egs_brachy, but indeed we need to coordinate with the egs_brachy team to see what adjustments are needed in EGSnrc to make egs_brachy a drop-in application in $EGS_HOME/.

Right now it is set up via the https://github.com/clrp-code/EGSnrc_CLRP fork, which contains the required tweaks and a submodule in $HEN_HOUSE/user_codes/egs_brachy. This module's egs_brachy subfolder is copied to the $EGS_HOME/egs_brachy during configuration. This is convoluted (perhaps no more so than EGSnrc itself!?). I would like to stick to a straightforward and uniform way to include third-party applications. Maybe the EGSnrc adjustments can be folded inside EGSnrc itslef, and a script provided within the egs_brachy repo to set it up from EGSnrc/third-party? For example, simply copy egs_brachy to $EGS_HOME and run make there?

@mchamberland, @MartinMartinov, what do you think? Is there a way you envision doing this?

@mchamberland
Copy link
Contributor

@ftessier It's been quite a while since I've looked at EGSnrc_CLRP so I forget what the additions are and if the current EGSnrc has caught up to it (e.g., with egsphant encoding and others). Maybe @MartinMartinov will have more input.

I'm open to the idea of a uniform way to install third-party applications, but I think we'll need to check with Dave and Rowan first (and I'll need to remember how to update our egs_brachy documentation!).

Not sure if we'll have this sorted out before you intend to release v2022, though.

@ftessier
Copy link
Member Author

ftessier commented Jul 26, 2022

Thanks for chiming in @mchamberland, I will mention it to Dave and Rowan. Yes, folding in egs_brachy is not for release 2022, but for develop soon afterwards I hope!

@MartinMartinov
Copy link
Contributor

It's only been 4 months, but already my memory is starting to slip. If I remember correctly, egs_brachy cannot be amalgamated into EGSnrc due to the differences in the way egs_glib (and egs_ndgeometry) handle egsphant files. In egs_brachy, density is defined voxel by voxel, whereas vanilla EGSnrc uses a ramp file for density assignment. It might have been something else, but this is what comes to mind.

@ftessier
Copy link
Member Author

ftessier commented Jul 26, 2022

Thanks @MartinMartinov, I will look at differences and opportunities to converge. Is anyone else taking over egs_brachy maintenance and development?

@ftessier
Copy link
Member Author

One way to resolve the issue is to provide a patch file within egs_brachy to git apply on top of, say, EGSnrc 2021 or any other version, and keep that on a separate branch in one's repository. Will take a look at that option.

@MartinMartinov
Copy link
Contributor

I'm unsure of who the go-to GitHub manager is now, unfortunately.

@rtownson
Copy link
Collaborator

rtownson commented Jul 27, 2022

It's only been 4 months, but already my memory is starting to slip. If I remember correctly, egs_brachy cannot be amalgamated into EGSnrc due to the differences in the way egs_glib (and egs_ndgeometry) handle egsphant files. In egs_brachy, density is defined voxel by voxel, whereas vanilla EGSnrc uses a ramp file for density assignment. It might have been something else, but this is what comes to mind.

By the way, this is what we want. It's been on the TODO list for a long time to get rid of the ramp file dependency for egsphant files.

@MartinMartinov
Copy link
Contributor

I think that would be huge. I think the RAMP files and the getMass/getVolume differences (until the recent merge) were the only changes I had to deal with when updating EGSnrc_CLRP with the new EGSnrc distros.

@mchamberland
Copy link
Contributor

mchamberland commented Jul 28, 2022

@MartinMartinov Thanks for chiming in, Martin!

@ftessier I volunteered myself to Dave and Rowan to be the official maintainer of the egs_brachy (and EGSnrc_CLRP) repo and they've agreed. Give me some time to re-orient myself in those repos and tidy up a few things, then we can see how to move forward.

@ftessier
Copy link
Member Author

Great info thanks! Also great if you can be the point person on this @mchamberland! If we can get back in sync so that egs_brachy installs cleaning over unmodified EGSnrc, that would prove useful, lest we fuel diverging versions!

@ftessier
Copy link
Member Author

Update README.

@ftessier
Copy link
Member Author

Squashed commits.

@ftessier ftessier merged commit 962cf5d into develop Oct 31, 2022
@ftessier ftessier deleted the add-thirdparty-3ddosetools branch October 31, 2022 20:22
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.

6 participants