Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with new optional parameter in 11_0_0_pre3 #27453

Closed
srimanob opened this issue Jul 7, 2019 · 21 comments
Closed

Issue with new optional parameter in 11_0_0_pre3 #27453

srimanob opened this issue Jul 7, 2019 · 21 comments

Comments

@srimanob
Copy link
Contributor

srimanob commented Jul 7, 2019

PdmV reported the issue in
https://its.cern.ch/jira/browse/CMSCOMPPR-7716

It seems to me that the issue comes from #27191 and #27399 which are introduced in 11_0_0_pre3. There is a change in python2 / python3 as mentioned in #27399 which issue reported before. Not clear why we see python2 in the injection process,

File "/cvmfs/cms.cern.ch/slc7_amd64_gcc700/external/python/2.7.14-pafccj/lib/python2.7/multiprocessing/pool.py", line 253, in map
return self.map_async(func, iterable, chunksize).get()
File "/cvmfs/cms.cern.ch/slc7_amd64_gcc700/external/python/2.7.14-pafccj/lib/python2.7/multiprocessing/pool.py", line 572, in get
raise self._value
AttributeError: '_ObsoleteParameter' object has no attribute 'value'

To test, we need to reach relval injection step:

eval `scram runtime -sh`
source /afs/cern.ch/cms/PPD/PdmV/tools/subSetupAuto.sh
runTheMatrix.py --what upgrade -l 10862.0 -t 8 -b 'phattest' --label 'phattest' --noCaf --wm init
runTheMatrix.py --what upgrade -l 10862.0 -t 8 -b 'phattest' --label 'phattest' --noCaf --wm force

Thanks.

@prebello @zhenhu

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 7, 2019

A new Issue was created by @srimanob Phat Srimanobhas.

@davidlange6, @Dr15Jones, @smuzaffar, @fabiocos, @kpedro88 can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@srimanob
Copy link
Contributor Author

srimanob commented Jul 7, 2019

@Dr15Jones @fabiocos
I am not sure that you are in the list of persons who can inject relvals, as the problem arises in the submission process only.

@fabiocos
Copy link
Contributor

fabiocos commented Jul 7, 2019

@srimanob I do not see how the changes in ConfigBuilder.py itself could cause problems, so your guess is the most reasonable one as those PRs touch the basic python behaviour behind the scenes. Nevertheless in the regular use of runTheMatrix.py itself we do not observe any problem, this is why I was asking more info about the reproducibility of the issue.

From the traceback the issue seems to come from

https://cmssdt.cern.ch/lxr/source/FWCore/ParameterSet/python/Types.py#0099

added in #27191 , which is required to provide value and does not.

@fabiocos
Copy link
Contributor

fabiocos commented Jul 7, 2019

assign core

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 7, 2019

New categories assigned: core

@Dr15Jones,@smuzaffar you have been requested to review this Pull request/Issue and eventually sign? Thanks

@Dr15Jones
Copy link
Contributor

@srimanob I 'hacked' some of the scripts and was able to find the actual origin of the problem. The useful trackback is

Importing the config, this may take a while... done.
Traceback (most recent call last):
  File "/cvmfs/cms.cern.ch/slc7_amd64_gcc700/cms/cmssw/CMSSW_11_0_0_pre3/bin/slc7_amd64_gcc700/runTheMatrix.py", line 339, in <module>
    ret = runSelected(opt)
  File "/cvmfs/cms.cern.ch/slc7_amd64_gcc700/cms/cmssw/CMSSW_11_0_0_pre3/bin/slc7_amd64_gcc700/runTheMatrix.py", line 41, in runSelected
    wfInjector.upload()
  File "/build/chrjones/production/CMSSW_11_0_0_pre3/python/Configuration/PyReleaseValidation/MatrixInjector.py", line 561, in upload
    d['ConfigCacheUrl']
  File "/build/chrjones/production/CMSSW_11_0_0_pre3/python/Configuration/PyReleaseValidation/MatrixInjector.py", line 539, in uploadConf
    cacheId = upload_to_couch_oneArg((filePath,labelInCouch,self.user,self.group,where))
  File "/build/chrjones/production/CMSSW_11_0_0_pre3/python/Configuration/PyReleaseValidation/MatrixInjector.py", line 34, in upload_to_couch_oneArg
    url=where)
  File "/afs/cern.ch/cms/PPD/PdmV/tools/wmcontrol/modules/wma.py", line 273, in upload_to_couch
    configCache.setPSetTweaks(makeTweak(loadedConfig.process).jsondictionary())
  File "/afs/cern.ch/cms/PPD/PdmV/tools/wmclient/v01/sw/slc5_amd64_gcc461/cms/wmcore/0.8.11a/lib/python2.6/site-packages/PSetTweaks/WMTweak.py", line 282, in makeTweak
    return maker(process)
  File "/afs/cern.ch/cms/PPD/PdmV/tools/wmclient/v01/sw/slc5_amd64_gcc461/cms/wmcore/0.8.11a/lib/python2.6/site-packages/PSetTweaks/WMTweak.py", line 253, in __call__
    for param in processParams if hasParameter(process, param) ]
  File "/afs/cern.ch/cms/PPD/PdmV/tools/wmclient/v01/sw/slc5_amd64_gcc461/cms/wmcore/0.8.11a/lib/python2.6/site-packages/PSetTweaks/WMTweak.py", line 150, in getParameter
    return lastParam.value()
  File "/cvmfs/cms.cern.ch/slc7_amd64_gcc700/cms/cmssw/CMSSW_11_0_0_pre3/python/FWCore/ParameterSet/Types.py", line 56, in __getattr__
    return object.__getattribute__ (self, name)
AttributeError: '_ObsoleteParameter' object has no attribute 'value'

So the problem is in the WMAgent code.

@Dr15Jones
Copy link
Contributor

There are 3 obsolete parameters in all of CMSSW

FWCore/ParameterSet/python/Config.py:    allowUnscheduled = cms.obsolete.untracked.bool,
FWCore/ParameterSet/python/Config.py:    emptyRunLumiMode = cms.obsolete.untracked.string,
FWCore/ParameterSet/python/Config.py:    makeTriggerResults = cms.obsolete.untracked.bool,

In the WM code PSetTweaks/WMTweak.py we can see that two of those are listed
https://github.com/dmwm/WMCore/blob/master/src/python/PSetTweaks/WMTweak.py#L42-L43

and it is ultimately from that list which leads to the eventual call to lastParam.value() which causes the error.

I see four possible ways around this

  1. PSetTweaks/WMTweak add a check that value is a valid function before making the call
  2. PSetTweaks/WMTweak removed the two obsolete parameters from its list
  3. We remove the use of cms.obsolete from the options PSet
  4. We extend _ObsoleteParameter to have a value() method which returns None

Ultimately this problem shows that, once again, we are unable to test CMSSW's interaction with WMAgent effectively during the IBs.

@Dr15Jones
Copy link
Contributor

I implemented option 4. It is unclear if this will have other consequences.

@fabiocos
Copy link
Contributor

fabiocos commented Jul 7, 2019

@Dr15Jones thank you, unfortunately I am afraid that besides what we may see in the PR test/IB we need to verify the interaction with WMAgent , I am not sure what the code do expect. @amaltaro options 1) but mostly 2) look simple to implement, what do you think?

@fabiocos
Copy link
Contributor

fabiocos commented Jul 8, 2019

#27454 seems to circumvent the problem according to @ahmad3213 . Do you need a new pre-release to inject relvals? Or do can you manage them with this addition for this round?

@ahmad3213
Copy link
Contributor

For the release validation process, we can manage with this addition (without pre-release).

@amaltaro
Copy link

amaltaro commented Jul 8, 2019

@Dr15Jones @fabiocos given that production might be handling requests/CMSSW where those parameters are not yet obsolete, option 1) seems to be our best choice.
However, if we don't have such releases around anymore, then getting rid of those two parameters is best, otherwise they get forgotten and stay there forever.

BTW, thanks for debugging and finding out that it actually comes from WMClient package. Honestly speaking, I'm not sure who maintain that package and how it gets shipped. I don't even see that 0.8.11a tag in the repository. We might have to add PPD/PdmV in the loop.

@Dr15Jones
Copy link
Contributor

@amaltaro I actually implemented 4) which appears to have solved the problem.

@Dr15Jones
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 8, 2019

This issue is fully signed and ready to be closed.

@Dr15Jones
Copy link
Contributor

Here is a better explaination as to why my change works. In WMTweak.getParameter, if the parameter in question is not available, the function returns None. If the parameter is available it calls parameter.value(). My change has an unset cms.optional and cms.obsolete return None from the call the value() which means WMTweak.getParameter returns the same value it did before when those parameters were not in the configuration. See

https://github.com/dmwm/WMCore/blob/master/src/python/PSetTweaks/WMTweak.py#L168

@amaltaro
Copy link

amaltaro commented Jul 8, 2019

Yep, they are equivalent. Thanks, Chris!

@zhenhu
Copy link
Contributor

zhenhu commented Jul 9, 2019

Hi @ahmad3213 , please inject one or two workflows to test it. If it works, let's stay with pre3 and inject all the rest relvals.

@ahmad3213
Copy link
Contributor

Hi @zhenhu , test was done already, please conversation at following PR
#27454
It worked fine. I will inject all with the mentioned change in above PR

@fabiocos
Copy link
Contributor

@srimanob @ahmad3213 I think this is solved now, and the PR fixing the problem will be part of CMSSW_11_0_0_pre4

@srimanob
Copy link
Contributor Author

Thanks All. I close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants