'Remove Section' GUI implementation

[louob20]

Appears to work, but please do check that this gives the desired results

I'm also unsure how to format the buttons in a nicer manner

[aris-mav]

This is a nice solution, but the results would not be 100% accurate, since the different x values might not match. We could instead use the original K matrix (as opposed to the one made right above) and compute Kf-g again for the residuals? That seems a bit more elegant and would give the most accurate residuals.

[louob20]

I've added the original ker_struct from the 1D invert method into the results. Calculating the residuals (as in the finding_alpha method, using g-Kf with g and K from the kernel struct) with the unchanged f leaves the residuals unchanged. However, changing f does give seemingly incorrect results. It's likely I'm just misunderstanding how ker_struct is used.

(even more seemingly incorrect than the last)

The following 2 are the plots of Kf, using the old and new K.

[aris-mav]

The values in the kernel structure sometimes are compressed by singular value decomposition to make the inversion faster and remove a bit of noise. I imagine that may introduce some confusion, since if the data is compressed, g will have a different size compared to x or y.
This is also why the x axis in the residuals uses "index" instead of time.

That's a bit confusing without looking at your code. Could you push it please?

[aris-mav]

Just tried the code on my machine, it works fine, well done!
Happy to merge once you consider the points above.

Also, I was thinking one more thing;
It might be good if we scale the updated distribution after the deletion, so that its integral is the same as the one in the original distribution.
Haven't tested it, but I imagine the fit will be much better after a section is deleted if we do that. Let me know whether you'd be keen to test that.

[louob20]

That sounds like a good idea, I will work on this.

In the current code there is no feature to find the integral of f is there?

[aris-mav]

That sounds like a good idea, I will work on this.

In the current code there is no feature to find the integral of f is there?

Great.
You're right, trapezoidal intergals would require external packages.
However, we don't necessarily care about the actual integral here.
Something like the sum all the values of the vector would be just as good as the integral in this case. As long as this measure is preserved, it would serve the same purpose as preserving the integral (since the x-axis would be the same before and after).

Perhaps we could also use the filter field to encode this information.
E.g., filter is all 1's at the beginning, but once some values are turned into 0's, the remaining 1's can get scaled up accordingly so that sum(f) == sum( filter .* f).
Hope that makes sense.

Of course, let me know if this, or anything else is unclear.

[louob20]

Got it, sounds good. Thanks!

[louob20]

Hi @aris-mav, I think I've figured out the issue. I have used create_kernel(res.seq, res.x, res.X) * res.f - res.y to find the residuals in the end.

Plotting create_kernel(res.seq, res.x, res.X) * res.f gives points that line up perfectly on the fitted curve and align with the res.y values. When removing an empty section, the residuals plot does change, however these must be the correct values.

Looking closely at the first index, having left f unchanged:

Original residuals plot,

Notice how the distance from the point to the fitted curve is ~0.005, but looking at the residual plot it predicts ~0.0001.

Updated residuals plot,

Now the distance from the point to the fitted curve has stayed the same at ~0.005, but the residual plot appears to be much more accurate, reading ~0.0058.

Using the updated method also gives sensible results when removing sections, unlike the original method, where residuals are very large initially and then quickly go to zero.
Additionally, if we plot K*f using the previous and updated method, the updated method gives a scatter plot of points arranged exponentially on the fitted curve, whereas the previous method gives odd results:

Original, res.ker_struct.K * res.f

Updated, create_kernel(res.seq, res.x, res.X) * res.f

[aris-mav]

That looks good, @louob20!
I'll also update how the original residuals are calculated, for consistency.

Just one thing, as mentioned in a previous comment, it would be good if the delete_selection! function only updates the filter field of the result structure. Also, filter_range might be a more appropriate name for it in this case.
This way, we can skip copying the original structure.
All the information will be there, and the new residuals/ yfit can be computed just before each plot (for both interactive and non-interactive plots).

[aris-mav]

Would you please enable “Allow edits from maintainers"?
This way I'll be able to push some changes here as well.

[louob20]

That looks good, @louob20! I'll also update how the original residuals are calculated, for consistency.

Just one thing, as mentioned in a previous comment, it would be good if the delete_selection! function only updates the filter field of the result structure. Also, filter_range might be a more appropriate name for it in this case. This way, we can skip copying the original structure. All the information will be there, and the new residuals/ yfit can be computed just before each plot (for both interactive and non-interactive plots).

So you have to apply the filter to the function only when you want to plot it?

[aris-mav]

So you have to apply the filter to the function only when you want to plot it?

That's what I was thinking, that would be consistent with the 2D case and it would preserve the original information.

[louob20]

@aris-mav I believe that last commit was all, if you would like to run the code before merging

[aris-mav]

It seems that the filter selection function still updates the residuals and the yfit directly on the structure.
Am I missing a commit perhaps?

[louob20]

I have changed it, should be good now.

[aris-mav]

Maybe the most elegant and consistent way to handle this would be following the same approach as the 2D case:
Add a filter field to the results struct, which can be a vector with 0's in the deleted region and 1's everywhere else.
Then, each time before plotting, we can plot filter .* f instead of f, K *( filter .* f ) .- yfit instead of r etc.

If so, the delete_range! function can be modified to delete_range(r::NMRInversions.inv_out_1D) (or another suitable name), and it can return the values above which need to be plotted, instead of modifying the original structure. This way, the original information is preserved, and the new information is computed in-place using the 'filter' field.

Can't think of a better way, but keen to hear your opinion as well.

[aris-mav]

Basically the delete button would only modify the filter field, and the reset button would turn it back to 1's.

[aris-mav]

The values in the kernel structure sometimes are compressed by singular value decomposition to make the inversion faster and remove a bit of noise. I imagine that may introduce some confusion, since if the data is compressed, g will have a different size compared to x or y.
This is also why the x axis in the residuals uses "index" instead of time.

That's a bit confusing without looking at your code. Could you push it please?

[aris-mav]

Good job @louob20 !