FullBinsOnly option for Rebin

[Arturs Bekasovs]

Muon scientists have an additional requirement for re-binning the data: if the the width of the last bin is less than the one requested (i.e. it is not full) it should be discarded.

No one from Mantid develop mailing list was against adding another option to Rebin algorithm called "FullBinsOnly" which would do exactly that. Should decide what it should for variable binning.

After that's done, both Muon interface and MuonLoad algorithm should be re-factored to use this option instead of doing it manually.

[Arturs Bekasovs]

Refs #8702. Add new property and initial test for it.

Changeset: fc67de5af4636a1046c31f853c248e46456b118a

[Arturs Bekasovs]

Refs #8702. Update axis creating method to include full_bins_only.

Add rigorous tests for it as well.

Changeset: a6e1b21bd0dc606f1313decfc2342738683821b4

[Arturs Bekasovs]

Refs #8702. Use the new parameter and check values in the test.

Changeset: 4badf4c0d5f171c2ab4550b076ff3db630a075cc

[Arturs Bekasovs]

Refs #8702. Add test for variable binning.

Changeset: f4902a88a8f2f036ea5791d0bd389ddb48d64d10

[Arturs Bekasovs]

Refs #8702. Modify Rebin description in Python Simple API test.

Changeset: 77add78a3c156ee326542ebcd5427d6bedeff1e2

[Arturs Bekasovs]

Refs #8702. Refactor MuonLoad and MuonAnalysis to use FullBinsOnly.

Changeset: fa20aa600ed665a31c39c690be4cd5fdfdca7f8f

[Arturs Bekasovs]

Tester:

Core functionality touched, senior developer please.

Rebin

Code-review changes in VectorHelper and make sure that when full_bin_only is false, the code logic should not be changed. It is a rather small piece of code.

Verify that unit tests for both VectorHelper and Rebin were added and are sensible.

You might find attached rebin_full_bins_only_test.py handy to play with Rebin.

MuonAnalysis and MuonLoad

Check that FullBinsOnly is now used instead of custom code in both places. Quickly verify the common sanity of the changes:

For MuonAnalysis:

1. In MuonAnalysis interface, load e.g. emu00006473 from AutoTestData.
2. On Settings tab:
   • Set Rebin params to Fixed of e.g. 2. 2 means that the Rebin will use the bin size of 2 * initial bin size as reported above the box.
   • Set Finish value to e.g. 10.
3. Plot something.
4. In plot workspace, check that the size of the last bin is as expected.
5. Do a few of such tests varying Rebin params and Finish value.

For MuonLoad:

1. Do the same check but using muon_load_full_bins_only_test.py instead of the interface, changing values of rebin_fixed_params and finish variables at the beginning of the script, and checking result workspace. Note: actual values might differ from the ones produced by MuonAnalysis; what's important here is the size of the last bin.

The system test for MuonLoad should now be in the develop branch of the systemtests repository, so as an additional check please verify that it is passing on test_*_develop builds.

[Arturs Bekasovs]

Should probably wait for system test to be added be fore modifying MuonLoad.

[Arturs Bekasovs]

Wrong ticket

[Arturs Bekasovs]

The system test for MuonLoad should now be in the develop branch of the systemtests repository, which is run against the develop branch. It is not necessary to wait until it gets to the master.

[Arturs Bekasovs]

Trac.

[Martyn Gigg]

I haven't looked at the code/played with it yet but my first observation is that the WIKI text could do with an extra section specifying exactly what happens to a dataset that has the FullBinsOnly option checked.

[Arturs Bekasovs]

Refs #8702. Update documentation.

Changeset: fddb6f1e65cc518fbcb2abb2d1c831d9c3dbeb74

[Arturs Bekasovs]

That was a useful suggestion.

[Arturs Bekasovs]

Wiki update is breaking the build. :-)

[Arturs Bekasovs]

Refs #8702. Fix the build.

It seems that wiki maker doesn't like internal links.

Changeset: 420bea0a5ffb763c4e342920fab35baeb9207b8c

[Martyn Gigg]

Generally the code changes look very nice. I've got a small comment about VectorHelper. I think the code

    // This coefficitent represents the maximum difference between the size of the last bin and all
    // the other bins.
    double lastBinCoef;

    if ( full_bins_only )
      // For full_bin_only, we want it so that last bin couldn't be smaller then the pervious bin
      lastBinCoef = 1.0;
    else
      // In general, we want the last bin not to be smaler than 25% of previous bin
      lastBinCoef = 0.25;

could be extracted to outside the while loop to avoid the repeated testing within the loop. We should also initialize the lastBinCoef to something as this is good practice. My suggestion would be

// This coefficient represents the maximum difference between the size of the last bin and all
// the other bins.
double lastBinCoef(0.25);
if ( full_bins_only )
{
  // For full_bin_only, we want it so that last bin couldn't be smaller then the pervious bin
  lastBinCoef = 1.0;
}

The documentation in Rebin.cpp is also good but in the example param strings for the FullBinsOption could we use commas for separating the values as that is what users are expected to do. There is also a minor typo in ''Param'' sttring which should be ''Param'' string :)

[Arturs Bekasovs]

Refs #8702. Use valid Rebin Param strings as examples and fix a typo.

Changeset: c128c93f5697a05e22c8e9c0b63513de11942ec4

[Arturs Bekasovs]

Refs #8702. Move the variable out of the while loop.

Changeset: 03f36311ab18cdf6a51d3f85a0eea5d5d29498ab

[Martyn Gigg]

Things look good now.

[Martyn Gigg]

Merge remote-tracking branch 'origin/feature/8702_rebin_full_bins_only'

Full changeset: ​ccc40cb1fd8457a2e26bc6d4b4cf67e76370913f

[Stuart Campbell]

This ticket has been transferred to github issue ​9546