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

Build template normalize weights #730

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

dyollb
Copy link
Contributor

@dyollb dyollb commented Nov 11, 2024

Fixes #729

This PR adds an option (default OFF) to re-normalize the contribution from each input image at a per-pixel level.
It also adds a helper method ones_like(img, pixeltype) along the lines of np.ones_like

@ntustison
Copy link
Member

Thanks for all this. I haven't had time to look but will do so asap. Re. the normalization---how much have you coordinated with the template building scrips in the core ANTs repo, specifically antsMVTC.sh and antsMVTC2.sh?

@dyollb
Copy link
Contributor Author

dyollb commented Nov 11, 2024

Thanks for all this. I haven't had time to look but will do so asap. Re. the normalization---how much have you coordinated with the template building scrips in the core ANTs repo, specifically antsMVTC.sh and antsMVTC2.sh?

not at all. I am not familiar with the scripts, but looking at them now I couldn't see the equivalent of the normalization I propose here. Basically, I am trying ensure the weighted sum up to one, for every pixel. As an example, if only half the images include the C3 vertebra, then without re-normalizing the intensity will be half of the "true average" intensity (for the C3 vertebra).

@ntustison
Copy link
Member

If you haven't coordinated with the original scripts, I'd be inclined to recommend that you implement any normalization locally for your purposes.

However, I think the modification of deleting the temporary files is a good idea.

@stnava
Copy link
Member

stnava commented Nov 11, 2024

that should be optional - not a default. I've not looked at your implementation but you would be "changing the method" if you added this. eg you are implying that there is an underlying "partial matching" problem here and that is somewhat outside the assumption of the method. so the implication goes all the way back to the design of the objective function for template building. not sure you want to get into that with what should be a simple pull request. maybe you could do that 2nd.

@dyollb
Copy link
Contributor Author

dyollb commented Nov 11, 2024

that should be optional - not a default. I've not looked at your implementation but you would be "changing the method" if you added this. eg you are implying that there is an underlying "partial matching" problem here and that is somewhat outside the assumption of the method. so the implication goes all the way back to the design of the objective function for template building. not sure you want to get into that with what should be a simple pull request. maybe you could do that 2nd.

I can split the PR so the temp file issue doesn't get mixed into the discussion.
Regarding the normalization question. I guess you could label it as partial matching, however unless you mask out adjacent body regions, you will surely always get some variability in the field of view. In my example above the C3 vetrebra is at the periphery of the region of interest. It just seems more correct to me, if the brightness does not decrease because of the variability in the field of view.

@stnava
Copy link
Member

stnava commented Nov 11, 2024

"more correct" is a different method. just keep this PR simple.

@dyollb
Copy link
Contributor Author

dyollb commented Nov 11, 2024

"more correct" is a different method. just keep this PR simple.

I'm sorry if this sounded like a criticism of your tool. I really appreciate your great work and that it is openly available. That is why I am enthusiastic about contributing back.

Update: I have split the PR

  • moved all temp file stuff to a different PR
  • made the behavior optional and disabled by default

It's also ok if you prefer that this implementation does not diverge from your original scripts and papers.

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.

build_template is darker in regions that are not always fully in field of view.
3 participants