Fix some exceptions of cms.Modifier #16570

makortel · 2016-11-14T10:48:07Z

While working on something else, I found out that when having a typo in the cms.Modifier parameters (i.e. trying to modify a non-existing parameter without an explicit type), the cms.Modifier machinery does not raise a helpful exception. This PR proposes fixes for those along unit tests (that demonstrate the "mistakes").

Without this PR, the first error case gives the following traceback

  File "FWCore/ParameterSet/python/Config.py", line 1194, in toModify
    temp(obj)
  File "FWCore/ParameterSet/python/Config.py", line 1124, in __call__
    _modifyParametersFromDict(params, self.__args, self._raiseUnknownKey)
  File "FWCore/ParameterSet/python/Mixins.py", line 626, in _modifyParametersFromDict
    keyDepth+"."+key)
  File "FWCore/ParameterSet/python/Mixins.py", line 639, in _modifyParametersFromDict
    errorRaiser(key)
TypeError: _raiseUnknownKey() takes exactly 1 argument (2 given)

and the second the following

  File "FWCore/ParameterSet/python/Config.py", line 1194, in toModify
    temp(obj)
  File "FWCore/ParameterSet/python/Config.py", line 1124, in __call__
    _modifyParametersFromDict(params, self.__args, self._raiseUnknownKey)
  File "FWCore/ParameterSet/python/Mixins.py", line 634, in _modifyParametersFromDict
    params[key].setValue(value)

Tested in 8_1_0_pre16, no changes expected.

makortel · 2016-11-14T10:48:15Z

@cmsbuild, please test

cmsbuild · 2016-11-14T10:48:23Z

A new Pull Request was created by @makortel (Matti Kortelainen) for CMSSW_8_1_X.

It involves the following packages:

FWCore/ParameterSet

@cmsbuild, @smuzaffar, @Dr15Jones, @davidlange6 can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @wddgit, @wmtan this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

makortel · 2016-11-14T10:49:10Z

FWCore/ParameterSet/python/Config.py

    _modifyParametersFromDict(params, self.__args, self._raiseUnknownKey)
    for k in self.__args.iterkeys():
        if k in params:
            setattr(obj,k,params[k])
        else:
            #the parameter must have been removed
            delattr(obj,k)
+  @staticmethod
  def _raiseUnknownKey(key):


Other option would have been to add self parameter, but since it would go unused I chose @staticmethod.

makortel · 2016-11-14T10:52:01Z

FWCore/ParameterSet/python/Config.py

@@ -1119,17 +1119,16 @@ def __call__(self,obj):
    for k in self.__args.iterkeys():
        if hasattr(obj,k):
            params[k] = getattr(obj,k)
-        else:
-            params[k] = self.__args[k]


I'm not 100 % sure if this is a correct fix. I first started to patch Mixins._modifyParametersFromDict() to raise an exception if params[key] does not have setValue attribute. But after staring the code a bit I started to wonder why self.__args is copied to params here when Mixins._modifyParametersFromDict() does effectively same thing.

Anyway, unit tests pass and (subset of) limited matrix workflows give exactly the same config dumps.

I'm guessing the copy was historic.

makortel · 2016-11-14T11:16:39Z

@cmsbuild, please test
(let's try again)

cmsbuild · 2016-11-14T11:16:50Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/16329/console

Dr15Jones · 2016-11-14T14:19:17Z

FWCore/ParameterSet/python/Config.py

@@ -1119,17 +1119,16 @@ def __call__(self,obj):
    for k in self.__args.iterkeys():
        if hasattr(obj,k):
            params[k] = getattr(obj,k)
-        else:
-            params[k] = self.__args[k]


I'm guessing the copy was historic.

Dr15Jones · 2016-11-14T14:19:32Z

+1

cmsbuild · 2016-11-14T14:19:46Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar

Dr15Jones · 2016-11-14T14:19:48Z

@makortel thanks for the fix

cmsbuild · 2016-11-14T17:38:01Z

-1

Tested at: af4d95b

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16570/16329/summary.html

I found follow errors while testing this PR

Failed tests: UnitTests

Unit Tests:

I found errors in the following unit tests:

---> test testJetMETCorrectionsType1MET had ERRORS

cmsbuild · 2016-11-14T17:38:03Z

Comparison job queued.

cmsbuild · 2016-11-14T18:50:15Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16570/16329/summary.html

Alternative comparison was/were failed for workflow(s):
21224.0

Dr15Jones · 2016-11-14T18:52:42Z

The unit test failure was actually from a DAS failure.

There was a problem while querying DAS with query 'file dataset=/RelValTTbar_13/CMSSW_7_4_0_pre6-PU50ns_MCRUN2_74_V0-v2/GEN-SIM-RECO | grep file.name'. Server reply was:
 {u'status': u'fail', u'mongo_query': {}, u'ctime': 0, u'timestamp': 1479137493.66, u'reason': u'request expired', u'data': []}
----- Begin Fatal Exception 14-Nov-2016 16:31:53 CET-----------------------
An exception of category 'ConfigFileReadError' occurred while
   [0] Processing the python configuration file named /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-11-13-2300/src/JetMETCorrections/Type1MET/test/corrMET_caloMet_cfg.py
Exception Message:
python encountered the error: <type 'exceptions.SystemExit'>
1
----- End Fatal Exception -------------------------------------------------

makortel added 2 commits November 14, 2016 11:41

Fix _ParameterModifier._raiseUnknownKey and add a unit test

a86df00

Fix _ParameterModifier for nonexisting scalar parameter

af4d95b

cmsbuild added this to the Next CMSSW_8_1_X milestone Nov 14, 2016

cmsbuild added comparison-pending core-pending orp-pending pending-signatures tests-pending labels Nov 14, 2016

makortel commented Nov 14, 2016

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Nov 14, 2016

Dr15Jones approved these changes Nov 14, 2016

View reviewed changes

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Nov 14, 2016

cmsbuild added tests-rejected and removed tests-started labels Nov 14, 2016

cmsbuild added comparison-available and removed comparison-pending labels Nov 14, 2016

davidlange6 merged commit 14cb428 into cms-sw:CMSSW_8_1_X Nov 15, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix some exceptions of cms.Modifier #16570

Fix some exceptions of cms.Modifier #16570

makortel commented Nov 14, 2016

makortel commented Nov 14, 2016

cmsbuild commented Nov 14, 2016

makortel Nov 14, 2016

makortel Nov 14, 2016

Dr15Jones Nov 14, 2016

makortel commented Nov 14, 2016

cmsbuild commented Nov 14, 2016 •

edited

Dr15Jones Nov 14, 2016

Dr15Jones commented Nov 14, 2016

cmsbuild commented Nov 14, 2016

Dr15Jones commented Nov 14, 2016

cmsbuild commented Nov 14, 2016

cmsbuild commented Nov 14, 2016

cmsbuild commented Nov 14, 2016

Dr15Jones commented Nov 14, 2016

Fix some exceptions of cms.Modifier #16570

Fix some exceptions of cms.Modifier #16570

Conversation

makortel commented Nov 14, 2016

makortel commented Nov 14, 2016

cmsbuild commented Nov 14, 2016

makortel Nov 14, 2016

Choose a reason for hiding this comment

makortel Nov 14, 2016

Choose a reason for hiding this comment

Dr15Jones Nov 14, 2016

Choose a reason for hiding this comment

makortel commented Nov 14, 2016

cmsbuild commented Nov 14, 2016 • edited

Dr15Jones Nov 14, 2016

Choose a reason for hiding this comment

Dr15Jones commented Nov 14, 2016

cmsbuild commented Nov 14, 2016

Dr15Jones commented Nov 14, 2016

cmsbuild commented Nov 14, 2016

cmsbuild commented Nov 14, 2016

cmsbuild commented Nov 14, 2016

Dr15Jones commented Nov 14, 2016

cmsbuild commented Nov 14, 2016 •

edited