-
Notifications
You must be signed in to change notification settings - Fork 0
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
Inaccurate FDPM amplitude and phase derivative values with respect to mu_a and mu_s' #42
Comments
Short course details, including slides and video, are linked here. The two referenced slides are here: |
@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? |
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. |
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. |
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. |
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. |
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. |
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. |
@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. |
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. |
I downloaded the msi we used for the short course and get the same results. So something is wrong in my settings. |
@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 |
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. |
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. |
With @hayakawa16's changes and some modifications to the GUI, we can now create the phase and amplitude plots: 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. |
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! |
…'s results posted Jan 14 on Issue #42
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. |
@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. |
@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. |
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? |
Oh wait do you mean it should look like the plot you post on Jan 16? |
@hayakawa16 Yes, either the one I posted on Jan 14th or the one you posted on Feb 16th |
I just tried inputs and I see distinct red plot. Shall we zoom? |
@hayakawa16 Thank you for getting me back on track, with the 4 different mus prime values, the plots look good: |
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
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
Here is what the GUI gives me for delPhi/delmua
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):
The text was updated successfully, but these errors were encountered: