cms-gem-daq-project / gem-plotting-tools

Repository for GEM commissioning plotting tools
GNU General Public License v3.0
1 stars 26 forks source link

[feature] Finding inflection point #231

Closed bregnery closed 5 years ago

bregnery commented 5 years ago

This new feature finds the inflection point (knee) in the SBit rate scan and marks this point with a vertical line on the SBit rate scan graph. But this functionality is only available for the channelor case.

Description

I created a new function, findInflectionPoint, and altered the sbitRateAnalysis function in utils/anautilities.py. The new function uses numpy.gradient along with some loops to find the inflection point. The altered sbitRateAnalysis draws a line in the sbit graph at the inflection point location. These changes are for the channelor case only.

Types of changes

Motivation and Context

We wanted to find the inflection point.

How Has This Been Tested?

I used a python virtual environment on my account on gem904daq01. The code was tested using sbit rate data for the channelor case.

Screenshots (if appropriate):

Checklist:

bdorney commented 5 years ago

Needs rebase ontop of develop

bdorney commented 5 years ago

Do something like:

git fetch -p --all
git checkout develop
git pull <name_of_central_remote> develop
get checkout <name_of_your_local_feature_branch>
git rebase -i develop

You may need to manually resolve conflicts (follow on screen instructions).

Once rebase is finished and all conflicts have been resolved you'll need to force update your remote (because your commit history will have changed):

git push -f <name_of_your_remote> <name_of_your_local_feature_branch>

Only do this last step once you're sure everything is working!!!

jsturdy commented 5 years ago

Do something like:

git fetch -p --all
git checkout develop
git pull <name_of_central_remote> develop
get checkout <name_of_your_local_feature_branch>
git rebase -i develop

I have tried to recommend/stress that people should not keep personal versions (from their forks) of certain branches (develop is one of them), if you want a develop branch, you should ensure that it tracks the cms-gem-daq-project/<repo> develop branch (likewise for other branches).

* c3c7979        (HEAD -> bregnery/feature/inflectionPoint) Prepare for pull request
* e7265c3        Channelor case is ready
* a63f1b7        Working on perchannel case
* 4eb556c        Trying to update to use TLine
* 93b12e6        Added Inflection point finder
| * 581e241      (gemdaq/develop) Feature Updated ana_scans.py and made most analysis routines importable (#225)
| * e8c3f02      updating DB query to VFAT prod DB to fix bug caused by relying only on max(RUN_NUMBER), fixes issue #229 (#230)
|/  
* 135f4d8        (bregnery/develop) New Feature: Plotting Time Series HV Plots from DCS Monitoring Files (#222)
git remote add gemdaq git@github.com:cms-gem-daq-project/gem-plotting-tools.git
git checkout -t gemdaq/develop
## then every time you want to ensure your features are up to date
git fetch -p --all
git checkout develop
git pull
git checkout <my feature branch>
git rebase -i develop

What it seems that you did was to rebase your commits on top of your develop branch (possibly the intermediate steps @bdorney outlined didn't succeed), so they are still conflicting and out of date.

bdorney commented 5 years ago

Indeed name_of_central_remote was meant to be from the cms-gem-daq-project.

jsturdy commented 5 years ago

Clearly problems with rebase + refactor indicate that we will require some more guidelines

jsturdy commented 5 years ago

I would like @bdorney's comment on how these additions should interplay with other tools, especially tools that would like to programatically extract the CFG_THR_ARM_DAC value corresponding to the knee, to understand if anything further would be required in this PR. After that, you will, need to rebase again... (and I don't yet know what the triage should be between this PR and #238, as I suspect there will be conflicts during either rebase)

bdorney commented 5 years ago

I would like @bdorney's comment on how these additions should interplay with other tools, especially tools that would like to programatically extract the CFG_THR_ARM_DAC value corresponding to the knee, to understand if anything further would be required in this PR.

I'm not sure I understand your question; the original idea behind this knee finding was to determine the lower limit on where to perform the ARM DAC calibration. But this is not extracted algorithmically presently. The script behind this takes a user input:

https://github.com/cms-gem-daq-project/sw_utils/blob/develop/scripts/calibrateArmDac.sh

After that, you will, need to rebase again... (and I don't yet know what the triage should be between this PR and #238, as I suspect there will be conflicts during either rebase)

Triage should just be to keep functions in required locations. If this is not clear I can fork from @bregnery and do the cleanup myself and then submit a PR to his branch.

bdorney commented 5 years ago

@mexanick anything outstanding on your side?