Ticket #6981 (closed: fixed)
Remove the Linear from the SANS reduction
Reported by: | Gesner Passos | Owned by: | Gesner Passos |
---|---|---|---|
Priority: | blocker | Milestone: | Release 2.6 |
Component: | SANS | Keywords: | |
Cc: | Blocked By: | #7182 | |
Blocking: | Tester: | Mathieu Doucet |
Description (last modified by Gesner Passos) (diff)
Executing the Reduction on SANS you get the following warning message:
CalculateTransmission started Linear is deprecated. Use Fit instead. CalculateTransmission successful, Duration 0.11 seconds
Change History
comment:2 Changed 7 years ago by Gesner Passos
- Milestone changed from Release 2.5 to Release 2.6
It will be moved to the next release, because moving to Fit will introduce differences to the resulting workspaces. They will make the system tests to fail. So, it requires some time...
comment:4 Changed 7 years ago by Karl Palmen
Linear is scheduled to be removed in release 2.7 according to ticket #7072
comment:5 Changed 7 years ago by Gesner Passos
- Blocked By 7182 added
It was found that some requirements from Linear were different from Fit. After some discussion, it was agreed that implementing #7182 allows us to use Fit algorithm.
But, the error from the Linear and Fit using function LinearBackground are different, the reason is that Linear, calculates the errors as:
err = sqrt(s02 + x2s12 + 2s012)
while fit calculates the erros as:
err = sqrt(s02 + x2s12)
Where s02 is the covariance of the first parameter(c0), s12 is the covariance of the second parameter(c1), and s012 is the covariance between these two parameters. For the liner equation:
y = c0 + c1 x
The fit does the simplification of considering the parameters independent, which is acceptable for the generic approach that fit offers. So, changing from Linear to Fit will cause us to update the systemtests that used to use Linear mainly because the errors are different right now.
comment:8 Changed 7 years ago by Gesner Passos
- Status changed from new to inprogress
Update nexus files to be processed with new CalculateTransmission
re #6981
Changeset: https://github.com/mantidproject/systemtests/commit/bcf6282b116d1dce28f9b2e67b2aca2493770689
comment:9 Changed 7 years ago by Gesner Passos
CalculateTransmission: substitute Linear by Fit
re #6981
Changeset: 120bc055459e512050f5f2080595e662b5e3b7e3
comment:10 Changed 7 years ago by Gesner Passos
- Description modified (diff)
Tester:
This ticket is composed by changes inside the systemtests and the mantid code (branches: feature/6981_from_linear_to_fit and feature/6981_remove_linear_calculate_transmission)
The aim is to remove the Linear child algorithm from CalculateTransmission. First of all, by inspection of the code you can see that Linear was removed.
The second step is to evaluate if the change from Linear to Fit is acceptable. This you have to do analyzing the comment:5. Are you happy with the difference in the error calculation introduced by the Fit algorithm? - Look: I'm giving the tester the responsibility to evaluate this issue because I do not feel quite comfortable in judging this issue.
The third step is to check that the execution of scripts that call CalculateTransmission will continue to work as expected. Reductions on SANS instruments use this algorithm.
Below you have a script that executes the reduction for SNS SANS instruments.
from reduction_workflow.instruments.sans.sns_command_interface import * config = ConfigService.Instance() config["facilityName"]='SNS' EQSANS(False) AppendDataFile("EQSANS_1466_event.nxs") SolidAngle() UseConfig(False) UseConfigTOFTailsCutoff(False) CombineTransmissionFits(True) UseConfigMask(False) SetBeamCenter(96.29, 126.15) TotalChargeNormalization(normalize_to_beam=False) DirectBeamTransmission("EQSANS_1466_event.nxs", "EQSANS_4061_event.nxs", beam_radius=3) ThetaDependentTransmission(True) Reduce1D() # Scale up to match correct scaling. Scale(InputWorkspace="EQSANS_1466_event_Iq", Factor=2777.81, Operation='Multiply', OutputWorkspace="EQSANS_1466_event_Iq")
For the sans instruments in ISIS, you can follow the instruction at: http://trac.mantidproject.org/mantid/ticket/7314#comment:4 .The reduction will occur nicely, and, as you expect, there is no more the warning message:
Linear is deprecated. Use Fit instead.
Finally, the system tests were changed in order to cope with the changes using Fit algorithm. Here, the first step is to evaluate the files that have changed. But, specially, to accept that the update on the EQSANSTransAPIv2.py file is acceptable ( https://github.com/mantidproject/systemtests/commit/bcf6282b116d1dce28f9b2e67b2aca2493770689)
Besides, all the systemtests that calls the CalculateTransmission must be passing on develop branch, before you can pass this ticket.
If you are happy with everything, than you can merge to the master both systemtests and mantid code.
comment:11 Changed 7 years ago by Gesner Passos
- Status changed from inprogress to verify
- Resolution set to fixed
comment:12 Changed 7 years ago by Mathieu Doucet
- Status changed from verify to verifying
- Tester set to Mathieu Doucet
comment:13 Changed 7 years ago by Mathieu Doucet
- Status changed from verifying to closed
Test that everything still works correctly. I also checked that the new reference files are close to the originals. The changes are tiny and orders of magnitude below the signal. So although I'm not a fan of changing reference data to accommodate code changes I'm going to pass the ticket :)
comment:16 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 7827