More usability improvements sw fitpowder #210

RichardWaiteSTFC · 2025-01-14T09:05:37Z

Description

Various bugs and small features:

Fix bug in scale factor estimation
Add new function to fit bg and scale to all data (fixing model)
Support any ndbase optimizer in fit_background
Add methods to set and clear background region
Add method to plot background region
Initialise background parameters with sensible values when switching background strategy
Handle edges/nans properly in avg. of error when taking 1D cuts
Support general polynomial background in Q and en
Use 2D data Q bins when replace with 1D cuts (previously just used nQ bins between qmin and qmax)
Update tutorial
Add method to export data into structs (or array of structs for 1D) - for fitbenchmarking
Add ability to set background parameters, bounds etc. by string label
Add method to print parameters nicely

To Test
(1) Run tutorial 39 file

Didn't break a test as bg is all zeros in unit test - have now added a constant bg so this bug would have been caught.

codecov-commenter · 2025-01-14T09:10:22Z

Codecov Report

❌ Patch coverage is 57.60369% with 92 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.42%. Comparing base (7e87dee) to head (6ae56d9).
⚠️ Report is 8 commits behind head on master.

Files with missing lines	Patch %	Lines
swfiles/sw_fitpowder.m	57.00%	92 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #210      +/-   ##
==========================================
+ Coverage   43.29%   43.42%   +0.13%     
==========================================
  Files         243      243              
  Lines       16383    16552     +169     
==========================================
+ Hits         7093     7188      +95     
- Misses       9290     9364      +74

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2025-01-14T09:12:49Z

Test Results

4 files ± 0 124 suites ±0 5m 44s ⏱️ +45s
851 tests +19 833 ✅ +19 18 💤 ±0 0 ❌ ±0
2 404 runs +76 2 368 ✅ +76 36 💤 ±0 0 ❌ ±0

Results for commit 6ae56d9. ± Comparison against base commit eacac5a.

This pull request removes 3 and adds 22 tests. Note that renamed tests count towards both.

sw_tests.unit_tests.unittest_ndbase_optimisers ‑ test_optimise_rosen_parameter_fixed_minimum_not_accessible_with([email protected])
sw_tests.unit_tests.unittest_ndbase_optimisers ‑ test_optimise_rosen_parameter_fixed_minimum_not_accessible_with([email protected])
sw_tests.unit_tests.unittest_sw_fitpowder ‑ test_set_background_strategy

sw_tests.unit_tests.unittest_ndbase_optimisers ‑ test_optimise_rosen_parameter_fixed_minimum_not_accessible_with_vary_arg([email protected])
sw_tests.unit_tests.unittest_ndbase_optimisers ‑ test_optimise_rosen_parameter_fixed_minimum_not_accessible_with_vary_arg([email protected])
sw_tests.unit_tests.unittest_sw_fitpowder ‑ test_add_1Dcuts_withs_qs_specified
sw_tests.unit_tests.unittest_sw_fitpowder ‑ test_export_data_1d
sw_tests.unit_tests.unittest_sw_fitpowder ‑ test_export_data_2d
sw_tests.unit_tests.unittest_sw_fitpowder ‑ test_export_data_2d_with_filename
sw_tests.unit_tests.unittest_sw_fitpowder ‑ test_fit_background_and_scale
sw_tests.unit_tests.unittest_sw_fitpowder ‑ test_fix_bg_param_with_array_of_names
sw_tests.unit_tests.unittest_sw_fitpowder ‑ test_set_background_strategy_to_indep
sw_tests.unit_tests.unittest_sw_fitpowder ‑ test_set_background_strategy_to_planar
…

♻️ This comment has been updated with latest results.

Also use same method to cut data in plotting 1D cuts of 2D data as when replacing data with 1D cuts. Updated tests

Now pfit and SigP have same shape

This is done so that there is always an E0 label regardless of background strategy

mducle

Looks good but I've got some change requests!

I really like the new parameter name setter and printer and the background region plotting options.

Also, do you think it might be better to split tutorial 39 into two? One with the basic fitting and another going into more depth about the background estimation?

mducle · 2025-06-25T17:26:47Z

+sw_tests/+unit_tests/unittest_sw_fitpowder.m

+            out.set_bg_parameters(2, 11, 1); % intercept = 4*2 + 3 for cut 1 
+            out.set_bg_parameters(2, 13, 2); % intercept = 5*2 + 3 for cut 2


According to the comment in line 85, the intercept index is 3 - but here you're changing parameter index 2 (which should slope_q, right?)

Thanks, just checked this and the comment on line 85 refers to the equivalent planar background (which are checked on line 91). The parameters being set here are for the individual 1D cuts - i.e. parameter 2 corresponds to E0 - this is what disp_params gives

--- BACKGROUND --- Label Index Cut Index Initial E1 1 1 1 E0 2 1 11 E1 1 2 1 E0 2 2 13

I will update the comment on line 85 to be clearer

@RichardWaiteSTFC bump...

+sw_tests/+unit_tests/unittest_sw_fitpowder.m

swfiles/sw_fitpowder.m

mducle · 2025-06-25T18:19:55Z

swfiles/sw_fitpowder.m

+       fbg_planar = @(en, modQ, p, npoly_modQ) polyval(p(npoly_modQ+1:end), en(:)) + ...
+                    polyval([reshape(p(1:npoly_modQ),1,[]), 0], modQ(:)'); % q^n,..q^1, en^n,...en^1, const


Just from a code-reading perspective, I think it's a bit more obvious if this is changed to have two separate vectors of coefficients for the E- and Q-dependence rather than one vector and an offset, e.g.:

fbg_planar = @(en, modQ, poly_E, poly_modQ) polyval(poly_E, en(:)) + ... polyval([poly_modQ(:)', 0], modQ(:)'); % q^n,..q^1, en^n,...en^1, const

where the constant value is in poly_E.

This would then need changes to lines 257:

obj.fbg = @(en, modQ, p) obj.fbg_planar(en, modQ, p(end-obj.npoly_en+1:end), p(1:numel(p)-obj.npoly_en));

and 281:

intercepts = obj.fbg_planar(0, modQs, bg_pars(obj.npoly_modQ+1:end), bg_pars(1:obj.npoly_modQ));

The lines using this function are more verbose though... so it's up to you!

PS. I'm not sure the code in line 257 is correct - see comment below.

Yep I think you're right, but would it be OK to address this in a separate PR?

Sure, I've created an issue for it so we don't forget...

swfiles/sw_fitpowder.m

tutorials/publish/tutorial39.m

This is only valid in some cases

mducle

Just the change to comments in the unit test... (and possibly the qs thing...) otherwise good to go!

mducle · 2025-09-08T07:50:56Z

swfiles/sw_fitpowder.m

+       fbg_planar = @(en, modQ, p, npoly_modQ) polyval(p(npoly_modQ+1:end), en(:)) + ...
+                    polyval([reshape(p(1:npoly_modQ),1,[]), 0], modQ(:)'); % q^n,..q^1, en^n,...en^1, const


Sure, I've created an issue for it so we don't forget...

mducle · 2025-09-08T07:56:33Z

+sw_tests/+unit_tests/unittest_sw_fitpowder.m

+            out.set_bg_parameters(2, 11, 1); % intercept = 4*2 + 3 for cut 1 
+            out.set_bg_parameters(2, 13, 2); % intercept = 5*2 + 3 for cut 2


@RichardWaiteSTFC bump...

swfiles/sw_fitpowder.m

mducle

Looks good now. Thanks.

RichardWaiteSTFC added 7 commits January 8, 2025 16:46

Fix bug in estimate_scale_factor - background already added to ycalc

a7e78cd

Didn't break a test as bg is all zeros in unit test - have now added a constant bg so this bug would have been caught.

Add new function to fit bg and scale parameters to all data

f7dd48e

Support any ndbase optimizer in fit_background

127156e

Add method to plot background region on figures

d04fe5c

Add function (and tests) to set background region

9aa01e4

Set sensible background when changing strategy

4c25403

Handle edges/nans properly in avg. of error when taking 1D cuts

2869cff

RichardWaiteSTFC added 11 commits February 7, 2025 14:42

Add method to clear background region

cc82c7f

Support general polynomial background in Q and en

91e673b

Add unit tests for setting polynomial order

4464c7f

Use 2D data Q bins when replace 1D cuts

0ad70f0

Also use same method to cut data in plotting 1D cuts of 2D data as when replacing data with 1D cuts. Updated tests

Set SigP to 0 for fixed parameters in lm4

df70190

Now pfit and SigP have same shape

Fix bug for 1D cuts now using 2D Q bins

40eca62

Update tutorial to include setting bg strategy and setting bg region

7a6a70e

Add method to export data in struct form

7b470aa

Fix rebin_powspec_to_1D_cuts bug causing test failure

f8e4567

Add unit tests for export_data method

17e2b1c

Update tutorial with changing polynomial order

484018d

RichardWaiteSTFC changed the title ~~More usability improvements sw fitpowder (DRAFT IN PROG)~~ More usability improvements sw fitpowder May 19, 2025

RichardWaiteSTFC added 7 commits June 2, 2025 16:00

Swap order of bg parameters (Q first) and add bg param labels

e934108

This is done so that there is always an E0 label regardless of background strategy

Update changing bg strategy tests with non-identical bg parameters

12657f6

Add ability to set background parameters, bounds etc. using string

718275e

Add tests for setting bg parameters etc. with string labels

4031e9f

Fix bug in background param label order

4dfae05

Add method to nicely print/display parameters

a7b4b0e

Update tutorial with parameter display

016b604

RichardWaiteSTFC mentioned this pull request Jun 5, 2025

Size mismatch in fastmode=true, hermit=false (no mex) spinwave calculation on MacOS #221

Closed

RichardWaiteSTFC force-pushed the more_usability_improvements_sw_fitpowder branch from 8ee1d09 to 016b604 Compare June 23, 2025 12:43

mducle requested changes Jun 25, 2025

View reviewed changes

RichardWaiteSTFC added 6 commits September 3, 2025 11:38

Stop scaling covariance by variance of residuals

f0c6c2a

This is only valid in some cases

Fix bug in getting qcens of cuts when using binning in 2D data

5263316

Improve plot formatting

6e687a1

Support char and cell of char in bg param setter

78177cf

Save file when export data

4734e78

Add check for integer npoly

8e0376c

RichardWaiteSTFC mentioned this pull request Sep 3, 2025

Support complex background masks such as excluding 'fan' of scattering dispersing from Bragg peak in sw_fitpowder #227

Open

Remove comment form tutorial docs

92bbcd3

mducle requested changes Sep 8, 2025

View reviewed changes

Support specifying 1D cuts with array of q-values and test

6ae56d9

mducle approved these changes Sep 9, 2025

View reviewed changes

RichardWaiteSTFC merged commit dc06a4b into master Sep 10, 2025
12 checks passed

		out.set_bg_parameters(2, 11, 1); % intercept = 4*2 + 3 for cut 1
		out.set_bg_parameters(2, 13, 2); % intercept = 5*2 + 3 for cut 2

		fbg_planar = @(en, modQ, p, npoly_modQ) polyval(p(npoly_modQ+1:end), en(:)) + ...
		polyval([reshape(p(1:npoly_modQ),1,[]), 0], modQ(:)'); % q^n,..q^1, en^n,...en^1, const

More usability improvements sw fitpowder #210

More usability improvements sw fitpowder #210

Uh oh!

Conversation

RichardWaiteSTFC commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

mducle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mducle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mducle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

RichardWaiteSTFC commented Jan 14, 2025 •

edited

Loading

codecov-commenter commented Jan 14, 2025 •

edited

Loading

github-actions bot commented Jan 14, 2025 •

edited

Loading