ConfigDB parsing/dumping using CMSSW_15_1_0_pre1 associated with a VPSet lead to incorrect python

[Dr15Jones]

          I strongly believe that ConfDb parsing gets confused by the new cms.PSetTemplate - there is no error message about Ignoring any unknown type, and somehow unlabelled PSets turn up as module parameters in ConfDb (when one adds new instances of affected plugins) and then when extracting the python code, eg:

process.RecoTauProducer = cms.EDProducer( "RecoTauProducer",
[...]
    buildNullTaus = cms.bool( False ),
    verbosity = cms.int32( 0 ),
    modifiers = cms.VPSet( 
      cms.PSet(  name = cms.string( "sipt" ),
        plugin = cms.string( "RecoTauImpactParameterSignificancePlugin" ),
         = cms.PSet( 
           = cms.PSet( 
            minTrackPt = cms.double( 0.5 ),
            maxTrackChi2 = cms.double( 100.0 ),
            maxTransverseImpactParameter = cms.double( 0.1 ),
            maxDeltaZ = cms.double( 0.4 ),
            maxDeltaZToLeadTrack = cms.double( -1.0 ),
            minTrackVertexWeight = cms.double( -1.0 ),
            minTrackPixelHits = cms.uint32( 0 ),
            minTrackHits = cms.uint32( 3 ),
            minGammaEt = cms.double( 1.0 ),
            minNeutralHadronEt = cms.double( 30.0 )
          ),
           = cms.PSet( 
            minTrackPt = cms.double( 1.0 ),
            maxTrackChi2 = cms.double( 100.0 ),
            maxTransverseImpactParameter = cms.double( 0.03 ),
            maxDeltaZ = cms.double( 0.2 ),
            maxDeltaZToLeadTrack = cms.double( -1.0 ),
            minTrackVertexWeight = cms.double( -1.0 ),
            minTrackPixelHits = cms.uint32( 0 ),
            minTrackHits = cms.uint32( 8 ),
            minGammaEt = cms.double( 1.5 )
          ),

which is not proper python. So the new feature of PSTemplate is problematic for ConfDB.

Originally posted by @Martin-Grunewald in #47630 (comment)

[cmsbuild]

cms-bot internal usage

[cmsbuild]

A new Issue was created by @Dr15Jones.

@Dr15Jones, @antoniovilela, @makortel, @mandrenguyen, @rappoccio, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[Dr15Jones]

Further comments from the PR
#47630 (comment)

I suspect the problem is that for a VPSet vector, it is assumed that the elements are PSets and those PSets do not have their own label/name. But now the VPSet contains in addition the entry/component template = cms.PSetTemplate as a component, which screws up this assumption.

[Dr15Jones]

Further comments from the PR
#47630 (comment)

I did a parsing (CMSSW_15_1_0_pre1) of a template into ConfDb, migrated a menu to it (existing instances stay fine) and then added a new instance of the RecoTauProducer, which appeared in ConfDb and in the extracted python as the above,
with the new nested parameters.

[Dr15Jones]

@Martin-Grunewald

When you say you added a new instance of the RecoTauProducer could you post exactly how you did that? Did you add a new _cfi file? Did RecoTauProducer already exist in the DB and it was the addition that failed? I'm trying to determine the starting point as my attempt to perform the actions (I think) CondDB is doing on the configurations are not showing any issues with the PSetTemplate.

[makortel]

assign core, hlt

[cmsbuild]

New categories assigned: core,hlt

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

[makortel]

Replying here to #47630 (comment) (@mmusich)

I suppose from here as the auto-generated cfi-s take precedence.

That isn't a cfi file and doesn't add anything to the Process if loaded into the Process (which AFAICT is what confdb is doing).

As far as I can tell, the only cfi file that uses the RecoTauProducer.py is
https://github.com/cms-sw/cmssw-cfipython/blob/CMSSW_15_1_0_pre1/RecoTauTag/RecoTau/combinatoricRecoTaus_cfi.py#L1-L5

Loading the combinatoricRecoTaus_cfi.py into python shows modifiers = cms.required.VPSet so a PSetTemplate in modifiers can not come through there. On the other hand, the builders parameter is set to a cms.VPSet() with a concrete PSet() and a template = cms.PSetTemplate()` (well, basically with the content of https://github.com/cms-sw/cmssw-cfipython/blob/CMSSW_15_1_0_pre1/RecoTauTag/RecoTau/RecoTauProducer.py)

[Martin-Grunewald]

I added the new plugin instance within the ConfDb GUI to an existing menu in ConfDb using the ConfDb template 15_1_0_pre1. The gui allows to add new module instances whose parameters get populated by what was found during parsing of that template. For the RecoTauProducer, ConfDb lists the following files as being used:

RecoTauProducer                                   RecoTauTag/RecoTau/combinatoricRecoTaus_cfi.py                                                                                                                                                          
RecoTauProducer                                   RecoTauTag/RecoTau/python/RecoTauCombinatoricProducer_cfi.py                                                  

The first one is in cfipython (so, generated), while the second one is an explicit cfi found in the standard ../python directory.

[makortel]

Curiously this part from the modifiers VPSet from the issue description

         = cms.PSet( 
           = cms.PSet( 
            minTrackPt = cms.double( 0.5 ),
            maxTrackChi2 = cms.double( 100.0 ),
            maxTransverseImpactParameter = cms.double( 0.1 ),
            maxDeltaZ = cms.double( 0.4 ),
            maxDeltaZToLeadTrack = cms.double( -1.0 ),
            minTrackVertexWeight = cms.double( -1.0 ),
            minTrackPixelHits = cms.uint32( 0 ),
            minTrackHits = cms.uint32( 3 ),
            minGammaEt = cms.double( 1.0 ),
            minNeutralHadronEt = cms.double( 30.0 )
          ),
           = cms.PSet( 
            minTrackPt = cms.double( 1.0 ),
            maxTrackChi2 = cms.double( 100.0 ),
            maxTransverseImpactParameter = cms.double( 0.03 ),
            maxDeltaZ = cms.double( 0.2 ),
            maxDeltaZToLeadTrack = cms.double( -1.0 ),
            minTrackVertexWeight = cms.double( -1.0 ),
            minTrackPixelHits = cms.uint32( 0 ),
            minTrackHits = cms.uint32( 8 ),
            minGammaEt = cms.double( 1.5 )
          ),

looks like the following part from builders.template https://github.com/cms-sw/cmssw-cfipython/blob/CMSSW_15_1_0_pre1/RecoTauTag/RecoTau/RecoTauProducer.py#L26-L51

        qualityCuts = cms.PSet(
          signalQualityCuts = cms.PSet(
            minTrackPt = cms.double(0.5),
            maxTrackChi2 = cms.double(100),
            maxTransverseImpactParameter = cms.double(0.1),
            maxDeltaZ = cms.double(0.4),
            maxDeltaZToLeadTrack = cms.double(-1),
            minTrackVertexWeight = cms.double(-1),
            minTrackPixelHits = cms.uint32(0),
            minTrackHits = cms.uint32(3),
            minGammaEt = cms.double(1),
            useTracksInsteadOfPFHadrons = cms.optional.bool,
            minNeutralHadronEt = cms.double(30)
          ),
          isolationQualityCuts = cms.PSet(
            minTrackPt = cms.double(1),
            maxTrackChi2 = cms.double(100),
            maxTransverseImpactParameter = cms.double(0.03),
            maxDeltaZ = cms.double(0.2),
            maxDeltaZToLeadTrack = cms.double(-1),
            minTrackVertexWeight = cms.double(-1),
            minTrackPixelHits = cms.uint32(0),
            minTrackHits = cms.uint32(8),
            minGammaEt = cms.double(1.5),
            useTracksInsteadOfPFHadrons = cms.optional.bool
          ),

with the cms.optional.bool being omitted, and the PSet labels being missing.

[Martin-Grunewald]

cms.optional and cms.required parameters would be ignored in ConfDb parsing (ConfDb parsing code does not recognise those types and thus ignores them alltogether)

[mmusich]

I wonder if #47642 could potentially create more troubles on top of this.

[makortel]

I wonder if #47642 could potentially create more troubles on top of this.

If ConfDB parsing ignores cms.obsolete (that has been available for hand-written configuration fragments since 2019 #27191), I'd except no more trouble (but I could be wrong).

[Martin-Grunewald]

OK, by trial and error, using an example producer cfi file like this:

import FWCore.ParameterSet.Config as cms

myprod1 = cms.EDProducer("MyProd1",
    a = cms.string("1"),
    b = cms.VPSet(
        cms.PSet(cc=cms.string("c")),
        cms.PSet(dd=cms.string("d")),
        cms.PSet(ee=cms.string("e")),
        template = cms.PSetTemplate(
            aa=cms.string("")
            ),
    )
)

I find that ConfDb parsing actually does ignore the template parameter nicely (and also the template parameter has to be the last inside the VPSet.)

So now I am confused but I can only say that perhaps the explicit cfi file in the python directory ../python, RecoTauTag/RecoTau/python/RecoTauCombinatoricProducer_cfi.py is somehow messed up, and I picked this unlucky example,
and is unrelated to PSetTemplate.

To test this:
Using only the generated combinatoricRecoTaus_cfi.py in parsing I get a proper RecoTauProducer with builders VPSet but no modifiers VPSet as that is cms.required so ignored, when adding an new instance in ConfDb
Using only the explicit RecoTauCombinatoricProducer_cfi.py in parsing I get the messed up python code.

[Martin-Grunewald]

Apologies for mixing this up!

[Dr15Jones]

@Martin-Grunewald thanks for the follow up!

I've actually been looking at RecoTauCombinatoricProducer_cfi.py for the past half-hour or so thinking this could be the origin. That code has a large number of indirections, even loading other packages to get stuff

https://github.com/cms-sw/cmssw/blob/master/RecoTauTag/RecoTau/python/RecoTauCombinatoricProducer_cfi.py

I'm wondering if the isolated way ConfDB loads that stuff leads to some unforeseen behavior.

[Martin-Grunewald]

Indeed the RecoTauProducer plugin in ConfDB template 14_2_0 is likewise screwed up! ConfDB uses also here:

RecoTauProducer                                   RecoTauTag/RecoTau/combinatoricRecoTaus_cfi.py                                                                                                                                                          
RecoTauProducer                                   RecoTauTag/RecoTau/python/RecoTauCombinatoricProducer_cfi.py                                                  

ie first the good/generated one, and then the bad/explicit one, and apparently combined the two to make a final template for the plugin.

[makortel]

The dumpPython() from RecoTauCombinatoricProducer_cfi.py has

    modifiers = cms.VPSet(
        cms.PSet(
            name = cms.string('sipt'),
            plugin = cms.string('RecoTauImpactParameterSignificancePlugin'),
            qualityCuts = cms.PSet(
                isolationQualityCuts = cms.PSet(
                    maxDeltaZ = cms.double(0.2),
                    maxDeltaZToLeadTrack = cms.double(-1.0),
                    maxTrackChi2 = cms.double(100.0),
                    maxTransverseImpactParameter = cms.double(0.03),
                    minGammaEt = cms.double(1.5),
                    minTrackHits = cms.uint32(8),
                    minTrackPixelHits = cms.uint32(0),
                    minTrackPt = cms.double(1.0),
                    minTrackVertexWeight = cms.double(-1.0)
                ),
                leadingTrkOrPFCandOption = cms.string('leadPFCand'),
                primaryVertexSrc = cms.InputTag("offlinePrimaryVertices"),
                pvFindingAlgo = cms.string('closestInDeltaZ'),
                recoverLeadingTrk = cms.bool(False),
                signalQualityCuts = cms.PSet(
                    maxDeltaZ = cms.double(0.4),
                    maxDeltaZToLeadTrack = cms.double(-1.0),
                    maxTrackChi2 = cms.double(100.0),
                    maxTransverseImpactParameter = cms.double(0.1),
                    minGammaEt = cms.double(1.0),
                    minNeutralHadronEt = cms.double(30.0),
                    minTrackHits = cms.uint32(3),
                    minTrackPixelHits = cms.uint32(0),
                    minTrackPt = cms.double(0.5),
                    minTrackVertexWeight = cms.double(-1.0)

I.e. the PSet element of the VPSet has an inner qualityCuts PSet, that has inner isolationQualityCuts and signalQualityCuts (in different order though, and there are other parameters between the two).

It looks to me as if the qualityCuts here would be replaced with the qualityCuts from the PSetTemplate in RecoTauTag.RecoTau.combinatoricRecoTaus_cfi.combinatoricRecoTaus.builders, but with PSet labels removed.

(all this might be just incidental and I'm chasing a red herring, this certainly should be independent of behavior in 14_2_0)

[Dr15Jones]

So I modified ConfdbLoadParamsFromConfigs141.py

• allowed the use of a 'dummy' DB class that just prints out the execute calls
• fixed whitelisting so I could quickly run just the Subsystem containing the problematic module

With these, I still can't get the system to parse both _cfi files as I get

process.extend(RecoTauTag.RecoTau.combinatoricRecoTaus_cfi)
(EDProducer RecoTauProducer) RecoTauTag.RecoTau.combinatoricRecoTaus
SELECT a.id, a.name, a.cvstag, a.id_pkg FROM u_moduletemplates a, u_modt2rele c  WHERE c.id_release=1 AND a.name = 'RecoTauProducer' AND c.id_modtemplate=a.id
...
process.extend(RecoTauTag.RecoTau.RecoTauCombinatoricProducer_cfi)
(EDProducer RecoTauProducer) RecoTauTag.RecoTau.combinatoricRecoTaus
Skipping the cfi for combinatoricRecoTaus because a template was already loaded for RecoTauProducer

indeed, both of the _cfi files declare the module label as combinatoricRecoTaus.

@Martin-Grunewald any idea on how I might force it to combine both?

[Martin-Grunewald]

I think you have to go through the code and set allowmultiplecfis=true everywhere.

[Dr15Jones]

I was able to get both _cfi.py files parsed by using the preferred command line option with

hlt-confdb/parser/ConfdbLoadParamsFromConfigs141.py -w RecoTauTag -i preferred -v 3

and

[cdj@cmslpc-el8-heavy02 src]$ cat preferred 
RecoTauTag/RecoTau RecoTauCombinatoricProducer_cfi.py

I'm having a hard time interpreting the results however

[Dr15Jones]

So I think my diagnostic printouts are showing the problem. In the output I see

The nesting is:
[('VPSet', 'modifiers', -1186), ('PSet', 'VPSet[1]', -1187)]
Added PSet  with superId = -1190
The first parameter for PSet modifiers[1].qualityCuts-1190 will be inserted with sequenceNb 0
Added PSet  with superId = -1191
The first parameter for PSet modifiers[1].qualityCuts.signalQualityCuts-1191 will be inserted with sequenceNb 0
                modifiers[1].qualityCuts.signalQualityCuts.minTrackPt   cms.double(0.5)
Added minTrackPt with paramId = -1192
The nesting is:
[('VPSet', 'modifiers', -1186), ('PSet', 'VPSet[1]', -1187), ('PSet', 'qualityCuts', -1190), ('PSet', 'signalQualityCuts', -1191)]
                modifiers[1].qualityCuts.signalQualityCuts.maxTrackChi2 cms.double(100.0)
Added maxTrackChi2 with paramId = -1193

but when I look at the SQL statements that are supposed to be derived from it, I do not see qualityCuts appear

INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','3','VPSet','modifiers','0','1','9','', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','2','PSet','','1','1','0','', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','string','name','2','1','0','sipt', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','string','plugin','2','1','1','RecoTauImpactParameterSignificancePlugin', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','2','PSet','','2','1','2','', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','2','PSet','','3','1','0','', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','minTrackPt','4','1','0','0.5', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','maxTrackChi2','4','1','1','100.0', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','maxTransverseImpactParameter','4','1','2','0.1', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','maxDeltaZ','4','1','3','0.4', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','maxDeltaZToLeadTrack','4','1','4','-1.0', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','minTrackVertexWeight','4','1','5','-1.0', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','uint32','minTrackPixelHits','4','1','6','0', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','uint32','minTrackHits','4','1','7','3', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','minGammaEt','4','1','8','1.0', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','minNeutralHadronEt','4','1','9','30.0', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','2','PSet','','3','1','1','', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','minTrackPt','4','1','0','1.0', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','maxTrackChi2','4','1','1','100.0', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','maxTransverseImpactParameter','4','1','2','0.03', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','maxDeltaZ','4','1','3','0.2', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','maxDeltaZToLeadTrack','4','1','4','-1.0', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','minTrackVertexWeight','4','1','5','-1.0', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','uint32','minTrackPixelHits','4','1','6','0', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','uint32','minTrackHits','4','1','7','8', :PLOB) PLOB= 
INSERT INTO u_modtelements (id_modtemplate,moetype, paramtype,name, lvl, tracked, ord, value, valuelob)  VALUES ('1','1','double','minGammaEt','4','1','8','1.5', :PLOB) PLOB= 

[Dr15Jones]

So here we have a PSet held by (name, value) by a PSet which is held by (name, value) by a PSet which is finally held by index by a VPSet. The names assigned to the two PSets are missing.

[Dr15Jones]

I found the problem. It occurs when we have a PSet in a PSet in a VPSet, e.g.

foo = cms.EDProducer("FooProd", 
                      vp = cms.VPSet( 
                               cms.PSet( 
                                   foo = cms.PSet(bar = cms.int32(1)), 
                                   fii = cms.string("blah") ) ) )

The label foo will be lost (really it is replaced with an empty string).

The offending line of code is
https://github.com/cms-sw/hlt-confdb/blob/b61ec727e92d66fa010db12dc118cb576232b2ad/parser/ConfdbLoadParamsFromConfigs141.py#L1391-L1392

I added a printout in that section and I see

Added VPSet modifiers with superId = -1186
                modifiers       VPSet[4]
RENAMING 'modifiers' to blank with psetname:VPSet[4]
Added PSet "" with superId = -1187
The first parameter for PSet modifiers[1]-1187 will be inserted with sequenceNb 0
                modifiers[1].name       cms.string('sipt')
Added "name" with paramId = -1188
The nesting is:
[('VPSet', 'modifiers', -1186), ('PSet', 'VPSet[1]', -1187)]
                modifiers[1].plugin     cms.string('RecoTauImpactParameterSignificancePlugin')
Added "plugin" with paramId = -1189
The nesting is:
[('VPSet', 'modifiers', -1186), ('PSet', 'VPSet[1]', -1187)]
RENAMING 'qualityCuts' to blank with psetname:modifiers[1]
Added PSet "" with superId = -1190
The first parameter for PSet modifiers[1].qualityCuts-1190 will be inserted with sequenceNb 0
RENAMING 'signalQualityCuts' to blank with psetname:modifiers[1].qualityCuts
Added PSet "" with superId = -1191
The first parameter for PSet modifiers[1].qualityCuts.signalQualityCuts-1191 will be inserted with sequenceNb 0
                modifiers[1].qualityCuts.signalQualityCuts.minTrackPt   cms.double(0.5)
Added "minTrackPt" with paramId = -1192
The nesting is:
[('VPSet', 'modifiers', -1186), ('PSet', 'VPSet[1]', -1187), ('PSet', 'qualityCuts', -1190), ('PSet', 'signalQualityCuts', -1191)]
                modifiers[1].qualityCuts.signalQualityCuts.maxTrackChi2 cms.double(100.0)
Added "maxTrackChi2" with paramId = -1193

[Dr15Jones]

unassign core

[Dr15Jones]

I did a pull request to cms-sw/hlt-confdb which adds a unit test to that code which demonstrates the problem and shows that other configuration extensions work just fine.

[Martin-Grunewald]

Thanks a lot for this Chris!