padsley / k600analyser

Code for the K600 analyser including plugin codes for silicon, clover and HAGAR data
1 stars 4 forks source link

Including more lineshape correction factors #146

Open LindsD opened 7 years ago

LindsD commented 7 years ago

Previously I dealt with adding more factors to the lineshape correction expression by adding them to the ODB. As Retief says, if I continue with this plan of action, I'll be moving backwards since we shouldn't rely on the ODB so much anymore.

I would like to be able to implement a lineshape correction of this form:

Xcorr = X - (a1*ThSCAT + a2*ThSCAT^2 + a3*ThSCAT^3 + a4*ThSCAT^4 + a5*ThSCAT^5 
                      + b1*(Y1+yoffset) + b2*(Y1+yoffset)^2 
                      + f1*ThSCAT*(X-x0) + f2*ThSCAT^2*(X-x0))

The current code covers the first line. So I need to modify things so that I can do the other lines. I've just made an issue to keep you informed of what I'm doing.

LindsD commented 7 years ago

Ok, so I have a question regarding the yoffset. So my lineshape correction before didn't actually have (Y1+yoffset) there. It was just Y1. I factored the yoffset in when it did CalcCorrX. So in my f-plane.c I had the following sections of code:

void CalcCorrX(Double_t X, Double_t Y, Double_t ThetaSCAT, Double_t *Xcorr)
{
   *Xcorr= X - (gates.a0xcorr*ThetaSCAT + gates.a1xcorr*ThetaSCAT*ThetaSCAT + gates.a2xcorr*ThetaSCAT*ThetaSCAT*ThetaSCAT 
        + gates.a3xcorr*ThetaSCAT*ThetaSCAT*ThetaSCAT*ThetaSCAT + gates.a4xcorr*ThetaSCAT*ThetaSCAT*ThetaSCAT*ThetaSCAT*ThetaSCAT + gates.b0xcorr*Y + gates.b1xcorr*Y*Y + gates.f0xcorr*ThetaSCAT*(X-gates.x0) + gates.f1xcorr*ThetaSCAT*ThetaSCAT*(X-gates.x0)) ;
}

and later:

CalcCorrX(X1pos - x1offset, (Y1+gates.yoffset), ThSCAT, &Xcorr);
   t_X1posC=Xcorr;

   t_Ex = CalcEx(Xcorr);

So can I not just do the same thing as in the second bit of code above for the section in main.c? If so, then do I still define the yoffset in Parameters.c and then make reference to it in the config file? Or do I define it within FocalPlane.c since that is where the x1offset is defined for each run/case?

But then I wouldn't really know what to do with the x0 in my lineshape expression. Ok so basically I think I'm not sure what the difference is between defining it within the lineshape CorrectionTerms (basically having a reference to it in FocalPlane.c) and only making reference to it in the main.c file later. I hope I have expressed my confusion in a way that you can understand.

PS. Sorry...

padsley commented 7 years ago

I don't know enough to comment on the precise details of what you are trying to do, but I'll answer what I can.

then do I still define the yoffset in Parameters.c and then make reference to it in the config file? Or do I define it within FocalPlane.c since that is where the x1offset is defined for each run/case?

If you are going to use the same yoffset for each run, then do it in Parameters.c in the usual fashion. If you are not but are going to change it run-by-run, you may need to hard-code it in. However, I'm generally not in favour of that.

If you want to do it properly, you should then define yoffset in Parameters.c, possibly as an array, and read in a text file which defines the yoffsets for each run within it. It might be worth spending some time thinking about how you want to do this...

padsley commented 7 years ago

and FWIW, we should do the same thing with x1offset - move the definition to Parameters.c and read a file in which defines the offsets for each run...

neveling commented 7 years ago

I agree with Phil. If this functionality goes into Parameters.c then it is useful for the whole k600analyser project. It will safe time for someone else in future.

On the suggestion that the x1offset should also go into Parameters.c, I agree. I will take that up

padsley commented 7 years ago

@LindsD are you making these changes on a particular branch? If so, I can probably pitch in if you have problems (but let me know which branch!)

Also, if you are going to be looking at this, please push the changes you've made so far to the branch to try to avoid version conflicts if you do need a hand.

LindsD commented 7 years ago

@neveling Well... I'm happy to try to take that project on if you'd like? I think it would be a good exercise.

(I just might need to consult with you guys sometimes)

padsley commented 7 years ago

Uh. Thinking about it, only one run is done at a time so the structure is actually fairly simple... make a x1offset and yoffset in Parameters.c with the correct read statements coming from some file defined in the config file.

Then you just search that file looking for the right run number and read in the offsets when the right number is found, otherwise you leave them as 0.

Actually... Make it the same input file with columns run_number, x1offset, y1offset, x2offset, y2offset (last two will probably always be 0). Simples.

LindsD commented 7 years ago

@padsley Yes, I'm working in my PR217 branch. I haven't made any changes yet. When it comes to coding, I like to think twice (actually a lot more than twice) before I actually make the changes. It needs to make sense in my head first. But I was thinking of diving in today. I will change Parameters.c first.

(Oh and just FY1. Basically the y offset is used to correct for where the beam was positioned when the data were taken. For a lot of my Neodymium data, the beam couldn't be centred as this caused more halo. The result is that the spectrum sits very high or very low on X1vsY1. The offset shifts it so that the spectrum is centred around Y1=0)

LindsD commented 7 years ago

@padsley Lol FY1 ... A pun I did not intend. FYI

padsley commented 7 years ago

The plan is this:

(0) Start your coding music. I recommend this: http://www.bbc.co.uk/programmes/b087tdb3

(1) Define the variables in Parameters.c.

(2) Define the extern variables in f-plane.c (and wherever else required).

(3) Add a tag in Parameters.c for the reading in of an offsets file and set up that reading in the config file.

(4) Do the actual reading-in section of the code with a new function in Parameters.c - should be able to copy this from how e.g. the ADC calibrations are read. During this reading, the first entry in the line (the run number) should be tested against the actual run number so that the offsets are set if and when the run number matches the entry in the file.

Does that make sense?

LindsD commented 7 years ago

Ok people :) I have been working on being able to do the position offset in the lineshape correction (Note: Not x1offset... I'll try that later)

I made a file which contains my proposed changes to the code. I haven't actually implemented them. I wanted to check my logic first... I've emailed it since this thingy won't let me attach a .c file (Made it .c since it is easier to read than a txt file). Please let me know what you think?

Thanking you muchly!

padsley commented 7 years ago

What's the status of this? I have some time on Saturday to look if you need a hand.

LindsD commented 7 years ago

Ok, so the status of this is:

The code can now implement a lineshape correction of the following form:

Xcorr = X - (a1*ThSCAT + a2*ThSCAT^2 + a3*ThSCAT^3 + a4*ThSCAT^4 + a5*ThSCAT^5 
                      + b1*(Y1+yoffset) + b2*(Y1+yoffset)^2 
                      + f1*ThSCAT*(X-x0) + f2*ThSCAT^2*(X-x0))

where f1, f2, ... and x0 are defined in Parameters.c and set in config.cfg. These changes have been merged into the master branch.

And with respect to @padsley 's comment:

and FWIW, we should do the same thing with x1offset - move the definition to Parameters.c and read a file in which defines the offsets for each run...

@neveling did this. We now have a file that is pointed to in config.cfg which contains a list of run numbers and associated x1offsets. I know it's been implemented in PR217 since I had to test the functionality. I think PR262 also has it.

PR260 required a similar thing for TOF offsets so @neveling has implemented those too.

I'm not sure if all of these changes have been merged into the master branch yet though. I don't think Retief has had time. If you and Retief think I can manage to merge them, I'm happy to do it. It just might take me longer than you two would take since I will double and triple check absolutely everything.

padsley commented 7 years ago

Mmmmkay.

We need to work out which changes are in which branches and what needs merging into the master. It sounds like there's a requirement that some collation is done as required changes are being made in different branches (217, 260, 262 - and 251 for the gamma stuff?) and we should probably try to make sure that all of these are up-to-date fairly soon before they diverge too much to easily merge.

Note that when the pull request is done, someone can set to do the merge and someone else to approve so if you want to do the merges @LindsD and then set someone else to approve them (as a back-stop - if you're double- and triple-checking everything will be fine :D) then you should do that.

neveling commented 7 years ago

Improvements to have TOF offsets and X1pos offsets read in via the config file are already implemented in the master branch. So far I've tried to always put improvements in the MASTER once is has shown to be working in one of the other branches. The only outstanding things still to make it to the MASTER branch, to my knowledge, relates to the PR251 gamma improvements. But to be safe, and as an excercise, lets merge PR217 to MASTER and see if some things were missed. Lindsay, I suggest you do the merge, then set both Phil and I to approve the merge.

LindsD commented 7 years ago

Ok so I just realised that Retief had actually asked me to test on the PR262 branch (sorry... too many project numbers and my brain got confused). So PR217 doesn't have the changes. So for @neveling 's suggestion:

But to be safe, and as an excercise, lets merge PR217 to MASTER and see if some things were missed. Lindsay, I suggest you do the merge, then set both Phil and I to approve the merge.

Should I just do this with PR262 and master?

Also... should I just manually copy over the new files from master into PR217? I don't think it's possible to merge in reverse without losing the master branch, right?

padsley commented 7 years ago

Uh. Don't do it yet. I still need to do the merge request for PR251 and master. Slow your roll, Dr Donaldson.