search

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

Bug in CalibrateThrDAC #246

Closed bregnery closed 5 years ago

bregnery commented 5 years ago

There is a bug in CalibrateThrDAC that breaks ana_scans.py armDacCal

Brief summary of issue

Lines 455-457 of commit #239 break the ana_scans.py armDacCal, now this function can only accept 1 argument. ana_scans.py armDacCal provides a pointer of 7 arguments.

Types of issue

Expected Behavior

successful completion of the following command:

ana_scans.py armDacCal -s 2019.08.27.14.17 --chamberConfig --medium -c

Current Behavior

Fails with exception:

Caught <type 'exceptions.TypeError'>: calibrateThrDAC() takes exactly 1 argument (7 given), terminating workers
Traceback (most recent call last):
  File "/data/bigdisk/users/bregnery/venvs/inflectionEnv/lib/python2.7/site-packages/gempython/scripts/ana_scans.py", line 247, in calArmDACParallelAna
    ).get(1800) # wait at most 30 minutes, this should be "relatively" quick
  File "/usr/lib64/python2.7/multiprocessing/pool.py", line 554, in get
    raise self._value
TypeError: calibrateThrDAC() takes exactly 1 argument (7 given)
Analysis Failed
Good-bye

Steps to Reproduce (for bugs)

  1. run ana_scans.py armDacCal -s 2019.08.27.14.17 --chamberConfig --medium -c

Possible Solution (for bugs)

Adjust the number of arguments.

Your Environment

bdorney commented 5 years ago

@AndrewLevin seems like the breaking change was introduced by your PR #239. Could you try to tackle this bug and resolve it for @bregnery?

AndrewLevin commented 5 years ago

Yes, so, in the pull request or the issue associated with https://github.com/cms-gem-daq-project/gem-plotting-tools/pull/239, @bdorney requested that the argument list be replaced by a namespace. So, I think we want to change ana_scans.py to use the namespace argument, do you agree @bregnery?

bdorney commented 5 years ago

Yes, so, in the pull request or the issue associated with #239, @bdorney requested that the argument list be replaced by a namespace. So, I think we want to change ana_scans.py to use the namespace argument, do you agree @bregnery?

Indeed; did not realize a breaking change would be created.

You need to pass something that can be handled by itertools.izip to the parallel analysis routine. Try to keep this in mind in designing your solution.

e.g. Passing a single Namespace object will not work because this is handling potentially multiple processes that will all need different input arguments.

Try to design your own solution here.