VirtualPhotonics / Vts.Gui.Wpf

WPF Application using the Vts library
Other
3 stars 0 forks source link

Inaccurate FDPM amplitude and phase derivative values with respect to mu_a and mu_s' #42

Closed vasangithub closed 5 months ago

vasangithub commented 11 months ago

The GUI does not produce accurate results for partial derivatives of FDPM amplitude and phase with respect to absorption and reduced scattering vs modulation frequency. I determined this by using the GUI to generate derivative estimates using a finite difference approach. Here is a screen shot of me getting the data for the 'numerator' of the finite difference for the derivative of amplitude with respect to absorption coefficient image

I imported this data into an Excel spreadsheet and estimated the derivative using finite difference. The results are in my talk in the short course: Lecture 14, slides 24 and 25

Here is what the GUI gives me for delA/delmua image

Here is what the GUI gives me for delPhi/delmua image

Similar inaccuracies exist for derivatives with respect to scattering. The problem exists for both the scaled MC solver and the diffusion solvers and also exist regardless of s-d separation.

Also the y-axis label should update when a derivative is shown and the units of the derivative should be specified.

Expected behavior Please see my plots in Lecture 14 slides 24 and 25 from the 2023 short course.

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

dcuccia commented 11 months ago

Short course details, including slides and video, are linked here. The two referenced slides are here:

Slide 24: image Slide 25: image

lmalenfant commented 11 months ago

@vasangithub I noticed in the screenshots, that you set the rho value to 90.9mm, which seems high to me. Usually, we see detector values in the range of 0.5mm and 9.5mm. Is this value correct? If so, what does this represent?

vasangithub commented 11 months ago

The error exists regardless of s-d separation. If you'd like, to debug you can use a more typical FDPM s-d separation such as 20mm.

hayakawa16 commented 11 months ago

I have distilled down this problem a bit. The ForwardSolverViewModel in ExecuteForwardSolver calls Vts/Factories/ComputationFactory.ComputeReflectance. ComputeReflectance calls another ComputeReflectance overload which calls GetForwardReflectanceFunc.GetDerivativeFunc. GetDerivativeFunc is in Vts.Modeling/Analyzers/NumericalDerivativeExtensions. I think this code all correct for the derivative of the real and imag parts of a complex solution domain. However, if the user toggles from Complex to Amp, the resulting plot is the amplitude of the derivatives (real and imag), not the derivative of the amplitude. Same thing for phase.

dcuccia commented 11 months ago

Thanks Carole! I think that sounds correct, then? The Complex/Amp/Phase toggle doesn't show different data, only different views/parts of the same data.

hayakawa16 commented 11 months ago

Well no it's not correct as far as a user like Vasan is concerned. He would like to toggle to Amplitude and have the plot of dA/dmua=d[sqrt(real^2+imag^2)]/dmua which is not equal to sqrt[(dreal/dmua)^2+(dimag/dmua)^2] (what is currently plotted). To enable this plot, I think we need to send the amplitude result to GetDerivativeFunc if the toggle is set to "Amplitude". I'm sure there is an intent to keep GUI code separate from VTS code as far as not sending VTS code a GUI toggle, so it will take some care to invoke this change.

vasangithub commented 11 months ago

Thanks all for the engagement here. Yes, Carole encapsulates well what I believe we should provide in the GUI. Most FDPM practitioners would like to see partial A/partial \mu_a or \partial \Phi/\partial \mu_a as well as the corresponding derivatives relative to \mu_s'. As a semantic point, what the GUI appears to be providing currently is not \partial A/\partial\mu_a but rather the Amplitude of \partial W/\partial\mu_a where W is the complex Fourier domain reflectance.

dcuccia commented 11 months ago

Ok - definitely understand that there's an important user need here that's not yet addressed. But what we have on the right hand side is a simple "Plot Toggle" to view the Re/Im or Amp/Phase of the same numbers/values, not a way of changing the underlying computation (on the left panel inputs). Like you say, this requires a change to the computation API first, and then to surface that new capability to the GUI. If we do make changes here, we should definitely do it equivalently across all domains, and be clear about what is being plotted.

lmalenfant commented 11 months ago

@dcuccia after our discussion today about being able to calculate the amplitude and phase derivatives using the original data, by calculating amplitude and phase and then passing either of these back to the reflectance calculation. Could you take a look at my branch to see if you think it's possible to do this calculation without changing the VTS code?

I changed the UI to allow amplitude and phase to be selected in the Forward Solver Panel, the plot toggle will appear if it is a complex plot and if the Model/Analysis Output is not R. I created an ExecuteForwardSolver override that passes the plot toggle (if it is Amp or Phase), it then calls ComputeReflectance with ForwardAnalysisType.R, my intention after that was to compute either amplitude or phase (depending on the toggle) and then pass those values back to ComputeReflectance this time with the correct ForwardAnalysisType.

This is where I am stuck, can we calculate amplitude and phase from the reflectance values (an array with real followed by imaginary) and if so what could we pass back to ComputeReflectance?

If you can think of a better solution, we are also open to ideas.

hayakawa16 commented 11 months ago

I created some overloads of ComputeReflectance that take in a Complex toggle setting so knows if the result should be real/imag, phase or amplitude. I have a kludge kind of working for 1 test cast, but have a general question: The charts from the short course have a y-axis of minus(partialA/partialmua) and then is on a log scale y-axis. I am indeed getting negative derivative values now, however without other changes to our GUI, we won't be able to change the y-axis title to include minus sign and negate the values to positive values so that they can be put on log scale and present results similar to those in the charts. As a first pass, negative derivatives will be plotted on a linear scale. Possibly @vasangithub has that data for us to use to verify our results.

hayakawa16 commented 10 months ago

I wanted to generate my own plots of the finite differences without a negative y-axis label and on a linear scale. I ran the WPF version 4.2 and tried to recreate the first plot on this issue. This is what I get.
ForVasan231213 Very different. Shape of plots different. As I increase absorption from 0.01/mm to 0.0105/mm, the amplitude plots reduce as they should. However, as I decrease scattering from 1/mm to 0.1/mm the plot amplitudes increase, not decrease as is shown in first plot above. Am I messing up a setting?

hayakawa16 commented 10 months ago

I downloaded the msi we used for the short course and get the same results. So something is wrong in my settings.

lmalenfant commented 10 months ago

@hayakawa16 Looking at the exported data, I was able to find the Rho values that were used to generate the first plot and I can now recreate that plot.

The Rho values were 9.9mm, 19.6mm, 47.6mm and 90.9mm

image

hayakawa16 commented 10 months ago

Thank you very much @lmalenfant for figuring this out!! I think I can use Vasan's spreadsheet now and determine a linear plot of the derivatives with a positive y-axis.

hayakawa16 commented 10 months ago

I took the spread sheet data and generated plots of the derivative of Amplitude with respect to mus and mua with a linear y-axis. I'm going to try to match these results first. I updated plots at 14:06. dAdmus_rho9p9 dAdmua_rho9p9

hayakawa16 commented 10 months ago

I am currently getting these. For the first plot, dAdmusp Vasan's value at ft=0 is -0.00016991, the branch GUI value at ft is -0.00017454, at ft=1.0 Vasan's value is -0.00013495, branch GUI is -0.00013946. For dAdmua, Vasan's value at ft=0 is -0.0102, branch GUI is -0.0104, at ft=1, Vasan's value is -0.0042, branch GUI is 0.0042. These differences are most likely due to the fact that the branch GUI takes a centered finite difference, and Vasan used a forward finite difference. dAdmusp dAdmua

hayakawa16 commented 10 months ago

I made a branch with the same name as this branch on the Vts. I'm going to push changes to both repositories. The code is far from final version.

lmalenfant commented 9 months ago

With @hayakawa16's changes and some modifications to the GUI, we can now create the phase and amplitude plots:

Phase image

Amplitude image

We added the Rho values to the plot data and changed the names of the derivatives, we removed the plot toggle on the plot tab.

hayakawa16 commented 8 months ago

With my latest edits on WPF and VTS branches 42a-inaccurate-fdpm... and with the inputs shown on @lmalenfant post on Jan 14, I can create plot below. Phase derivative max value isn't at 0, however Amplitude derivative looks correct. I'll keep at it, but I'm close! Screenshot_240216_dPhasedMua Screenshot_240216_dAmplitudedMua

hayakawa16 commented 8 months ago

I removed the phase wrapping check GenerateComplexDerivativePlot, i.e.: if (y < 0) { y += 360; } and now get these results which agree with above Screenshot_240216_dPhasedMua_nochecknegphase

hayakawa16 commented 8 months ago

I have verified the derivatives of real and imag for mus' derivatives with this code and phase and amplitude results look correct (do we have any plots to use to verify?). I think I'm at a point to have others try the code. Note that both WPF 42a-... and Vts 42a-... branches need to be pulled and the WPF solution needs to remove existing reference to NuGet Vts and replace with 42a-.. branch Vts.

lmalenfant commented 7 months ago

@hayakawa16 Upon reviewing the plots, I am seeing a slight discrepancy with the amplitude plots, phase looks good but when I switch to amplitude the red plot is missing.

image

lmalenfant commented 7 months ago

@hayakawa16 Just to clarify, the red plot is not missing, it is identical to the blue one so it is hidden, this was not the case previously.

As a side note, I just found a really nice feature with OxyPlot, if you click on the legend item it grays out and it is removed from the plot view so if you click the blue legend item, you will see the red plot. Click it again to bring it back.

hayakawa16 commented 7 months ago

Hi @lmalenfant would it be possible to show what the current GUI looks like so I know what you mean that the red plot can be seen?

hayakawa16 commented 7 months ago

Oh wait do you mean it should look like the plot you post on Jan 16?

lmalenfant commented 7 months ago

@hayakawa16 Yes, either the one I posted on Jan 14th or the one you posted on Feb 16th

hayakawa16 commented 7 months ago

I just tried inputs and I see distinct red plot. Shall we zoom?

lmalenfant commented 7 months ago

@hayakawa16 Thank you for getting me back on track, with the 4 different mus prime values, the plots look good:

image

image