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

Configuration parameter copy and modify functions extended #13244

Merged
merged 2 commits into from Feb 11, 2016

Conversation

Dr15Jones
Copy link
Contributor

The clone and Modifier.toModify methods have been extended to allow further modifications to the parameters after they have been copied.

  1. Specify None as the value of the parameter will cause the parameter to be removed from the configuration
  2. Parameters within embedded PSets can now be modified using a python dict as the value. The keys of the dictionary are the parameter
    names and the values are the new values to use.
    The clone and Modifier.toModify both call the same helper function to do the work of modifying the copied parameters.

This also fixed the problem with the previous pull request for this feature.

The `clone` and `Modifier.toModify` methods have been extended to allow further modifications to the parameters after they have been copied.
1) Specify `None` as the value of the parameter will cause the parameter to be removed from the configuration
2) Parameters within embedded PSets can now be modified using a python dict as the value. The keys of the dictionary are the parameter
    names and the values are the new values to use.
The `clone` and `Modifier.toModify` both call the same helper function to do the work of modifying the copied parameters.
The reimplementation of the Modifier accidentally lost the ability to specify a new parameter when doing the modification. This adds that feature back.
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @Dr15Jones (Chris Jones) for CMSSW_8_0_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, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@Dr15Jones
Copy link
Contributor Author

I tested this with #13216 by running runTheMatrix.py -l 5.1,140.1 where those two workflows exhibited the two different failures seen in the IB. With this pull request the jobs ran fine.

@Dr15Jones
Copy link
Contributor Author

please test

@Dr15Jones
Copy link
Contributor Author

+1

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

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

davidlange6 added a commit that referenced this pull request Feb 11, 2016
Configuration parameter copy and modify functions extended
@davidlange6 davidlange6 merged commit e0fe1de into cms-sw:CMSSW_8_0_X Feb 11, 2016
@Dr15Jones
Copy link
Contributor Author

The test aborted because it timed out

@Dr15Jones Dr15Jones deleted the extendedPSetModification2 branch February 15, 2016 16:08
@makortel
Copy link
Contributor

@Dr15Jones It looks like PSet.clone() does not work with dict arguments. Example (tested in CMSSW_8_1_X_2016-02-22-1100):

$ cat foo.py
import FWCore.ParameterSet.Config as cms
p1 = cms.PSet(anInt = cms.int32(1), a = cms.PSet(b = cms.int32(1)))
p2 = p1.clone(a = dict(b = 2))
print p2.a.b.value()
$ python foo.py
Traceback (most recent call last):
  File "foo.py", line 3, in <module>
    p2 = p1.clone(a = dict(b = 2))
  File .../CMSSW_8_1_X_2016-02-22-1100/python/FWCore/ParameterSet/Types.py", line 653, in clone
    myparams[key].setValue(value)
AttributeError: 'PSet' object has no attribute 'setValue'

@Dr15Jones
Copy link
Contributor Author

#13526

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

Successfully merging this pull request may close these issues.

None yet

4 participants