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

Testing/add etp and add configuration #20

Merged
merged 26 commits into from
Nov 2, 2023

Conversation

etpeterson
Copy link
Contributor

No description provided.

@oliverchampion
Copy link
Collaborator

Hey! Several questions:

What is the reason for now initializing the code for every voxel?:
Adding
self.initialize(bounds, initial_guess, fitS0, thresholds, prior_in)
to the ivim_fit routine, instead of running it in the super?
I imagine typically you want to fit a full image with similar settings. Then, if you want to change bounds, you could run initialize again after fitting the image?

@oliverchampion
Copy link
Collaborator

Moreover, the Bayesian approach needs an initial guess that is reasonable. This is now taken from the segmented fit. If the segmented fit is broken, it is expected that the Bayesian will not work well. :)

@oliverchampion
Copy link
Collaborator

I think for the Bayesian wrapper you can remove line 77 " fit_results=fit_results+(1,)"

@oliverchampion
Copy link
Collaborator

Unfortunately, I cannot run the tests on segmented fit without the json file (which is now being ignored by .gitignore).
My expectation is that the bounds implemented for my segmented fit are not working well (i.e. not well translated to the individual fits). Does the segmented fit work for the fit bounds given in my wrapper?

@oliverchampion
Copy link
Collaborator

I think I've fixed the errors in segmented and bayesian. Still confused about the initialisation.

@oliverchampion oliverchampion merged commit 21258ab into main Nov 2, 2023
24 checks passed
@etpeterson etpeterson deleted the testing/add_ETP_and_add_configuration branch November 6, 2023 23:10
etpeterson pushed a commit to etpeterson/TF2.4_IVIM-MRI_CodeCollection that referenced this pull request Jul 19, 2024
…uration

Testing/add etp and add configuration
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.

3 participants