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.

Sign up for GitHub

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

Add new ProcessBlock feature to the Framework #30117

Merged
merged 25 commits into from
Jul 10, 2020
Merged

Add new ProcessBlock feature to the Framework #30117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
af3125e
Add ProcessBlock feature to Framework
wddgit Mar 16, 2020
b28794d
Use std::array in ProductRegistry
wddgit Jun 10, 2020
9edfdd1
Use std::array in BranchType implementation
wddgit Jun 10, 2020
a295a63
Remove unnecessary private access specifiers
wddgit Jun 10, 2020
f3a2b00
Insert correct principal
wddgit Jun 11, 2020
aa0a0d4
Modernize, replace typedef with using
wddgit Jun 11, 2020
c4bef3b
Only perform find in fillPrincipal if needed
wddgit Jun 11, 2020
7efbcd6
Use enum argument instead of bool
wddgit Jun 12, 2020
3bbae12
Add a comment
wddgit Jun 12, 2020
11124c2
Test order of transitions more carefully
wddgit Jun 12, 2020
5e63c99
Add printout to sh script, test names
wddgit Jun 12, 2020
10e6854
Replace throws with asserts
wddgit Jun 16, 2020
5db1a14
Add makeNameArray function
wddgit Jun 16, 2020
fe579de
Use range for loop and make_unique
wddgit Jun 16, 2020
811a437
Change enum name to BranchActionProcessBlockInput
wddgit Jun 17, 2020
fa961fe
Change override to final
wddgit Jun 17, 2020
97406bb
Use loop in makeNameArray
wddgit Jun 17, 2020
a8bfbf5
Add curly braces for clarity
wddgit Jun 19, 2020
da0af14
Remove extra space
wddgit Jun 19, 2020
c7711b5
Initialize with BeginProcessBlock
wddgit Jun 19, 2020
9ac7bbe
Abilities WatchProcessBlock InputProcessBlockCache
wddgit Jun 24, 2020
41823e5
Resolve logical conflicts after PR rebase
wddgit Jun 25, 2020
9a21b34
Improve interface in globalTransitionAsync.h
wddgit Jun 29, 2020
fb30b13
Test modes Runs and RunsAndLumis
wddgit Jun 29, 2020
3c64a1b
Add enum value Transition::AccessInputProcessBlock
wddgit Jul 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions DataFormats/Provenance/interface/BranchDescription.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ namespace edm {

BranchDescription(BranchDescription const& aliasForBranch,
std::string const& moduleLabelAlias,
std::string const& poruductInstanceAlias);
std::string const& productInstanceAlias);

~BranchDescription() {}

Expand Down Expand Up @@ -142,10 +142,10 @@ namespace edm {
// This is set if and only if produced_ is true.
std::string moduleName_;

// The branch name, which is currently derivable fron the other attributes.
// The branch name, which is currently derivable from the other attributes.
std::string branchName_;

// The wrapped class name, which is currently derivable fron the other attributes.
// The wrapped class name, which is currently derivable from the other attributes.
std::string wrappedName_;

// For SwitchProducer alias, the label of the aliased-for label; otherwise empty
Expand Down
4 changes: 0 additions & 4 deletions DataFormats/Provenance/interface/BranchType.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ namespace edm {

std::string const& BranchTypeToProductProvenanceBranchName(BranchType const& BranchType);

std::string const& BranchTypeToMajorIndexName(BranchType const& branchType);

std::string const& BranchTypeToMinorIndexName(BranchType const& branchType);

std::ostream& operator<<(std::ostream& os, BranchType const& branchType);

namespace poolNames {
Expand Down
27 changes: 3 additions & 24 deletions DataFormats/Provenance/interface/ProductRegistry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ namespace edm {

ProductRegistry();

// A constructor from the persistent data memebers from another product registry.
// A constructor from the persistent data members from another product registry.
// saves time by not copying the transient components.
// The constructed registry will be frozen by default.
explicit ProductRegistry(ProductList const& productList, bool toBeFrozen = true);
Expand Down Expand Up @@ -132,35 +132,14 @@ namespace edm {
Transients();
void reset();

std::shared_ptr<ProductResolverIndexHelper const> eventProductLookup() const {
return get_underlying_safe(eventProductLookup_);
}
std::shared_ptr<ProductResolverIndexHelper>& eventProductLookup() {
return get_underlying_safe(eventProductLookup_);
}
std::shared_ptr<ProductResolverIndexHelper const> lumiProductLookup() const {
return get_underlying_safe(lumiProductLookup_);
}
std::shared_ptr<ProductResolverIndexHelper>& lumiProductLookup() {
return get_underlying_safe(lumiProductLookup_);
}
std::shared_ptr<ProductResolverIndexHelper const> runProductLookup() const {
return get_underlying_safe(runProductLookup_);
}
std::shared_ptr<ProductResolverIndexHelper>& runProductLookup() { return get_underlying_safe(runProductLookup_); }

bool frozen_;
// Is at least one (run), (lumi), (event) persistent product produced this process?
std::array<bool, NumBranchTypes> productProduced_;
bool anyProductProduced_;

edm::propagate_const<std::shared_ptr<ProductResolverIndexHelper>> eventProductLookup_;
edm::propagate_const<std::shared_ptr<ProductResolverIndexHelper>> lumiProductLookup_;
edm::propagate_const<std::shared_ptr<ProductResolverIndexHelper>> runProductLookup_;
std::array<edm::propagate_const<std::shared_ptr<ProductResolverIndexHelper>>, NumBranchTypes> productLookups_;

ProductResolverIndex eventNextIndexValue_;
ProductResolverIndex lumiNextIndexValue_;
ProductResolverIndex runNextIndexValue_;
std::array<ProductResolverIndex, NumBranchTypes> nextIndexValues_;

std::map<BranchID, ProductResolverIndex> branchIDToIndex_;

Expand Down
2 changes: 1 addition & 1 deletion DataFormats/Provenance/interface/ProductResolverIndexHelper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ namespace edm {
// the arguments or a set of matching indexes using the Matches
// class. A returned index can have a value that indicates that it
// is invalid or ambiguous and the client should check for these
// values before using the index (see ProductIndexHolder.h).
// values before using the index (see ProductResolverIndex.h).

// If no matches are found or the ProductResolverIndexHelper
// has not been frozen yet, then an invalid index or a Matches
Expand Down
132 changes: 61 additions & 71 deletions DataFormats/Provenance/src/BranchType.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
#include "DataFormats/Provenance/interface/BranchType.h"

#include <array>
#include <cassert>
#include <cstddef>
#include <iostream>

namespace edm {
Expand All @@ -16,52 +20,41 @@ namespace edm {
std::string const branchEntryInfo = "BranchEntryInfo"; // backward compatibility
std::string const productProvenance = "ProductProvenance";

// Prefixes
std::string const run = "Run";
std::string const lumi = "LuminosityBlock";
std::string const event = "Event";

// Trees, branches, indices
std::string const runs = run + 's';
std::string const lumis = lumi + 's';
std::string const events = event + 's';
std::array<std::string, NumBranchTypes> const branchTypeNames{{"Event", "LuminosityBlock", "Run", "ProcessBlock"}};
std::size_t constexpr eventLumiRunSize = 3;
using NameArray = std::array<std::string, eventLumiRunSize>;

std::string const runMeta = run + metaData;
std::string const lumiMeta = lumi + metaData;
std::string const eventMeta = event + metaData;
NameArray makeNameArray(std::string const& postfix) {
static_assert(InEvent == 0);
static_assert(InLumi == 1);
static_assert(InRun == 2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now at least it won't compile if the values are changed. On the other hand, a loop would work automatically with value changes.

(this discussion makes me wonder if we have implicit assumptions on their numerical values somewhere else)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the version of this file in the current master release, there is no assumption about the enum values. In other files, especially in the IOPool subsystem, such assumptions already exist in the code (and one might find them other places if one looked).

When converting to std::array and using the BranchType as an index, I created the dependence. First here:

std::array<std::string, NumBranchTypes> const branchTypeNames{{"Event", "LuminosityBlock", "Run", "ProcessBlock"}};

Personally, I think the current version in the PR is good enough. It is simple and easy to read. I doubt the enum values will ever be reordered. If you want I could change initialization of the above array so that the enum values could be reordered but still restricted to 0, 1, 2. If we want to support arbitrary values like InEvent = 100, InLumi = 101, and InRun = 102, then I would need to use something more like maps or add an additional layer of indirection. Note that there is also a presumption in the file that InProcess is 3. Most of the arrays have size 3 (run/lumi/event and no processBlock). Another alternative would be to remove the std::array's and go back to old way of implementing this.

static_assert(InProcess == 3);
NameArray ret;
for (auto i = 0U; i != eventLumiRunSize; ++i) {
ret[i] = branchTypeNames[i] + postfix;
}
return ret;
}

std::string const runInfo = run + "StatusInformation"; // backward compatibility
std::string const lumiInfo = lumi + "StatusInformation"; // backward compatibility
std::string const eventInfo = event + "StatusInformation"; // backward compatibility
const NameArray treeNames{makeNameArray(std::string("s"))};

std::string const runAuxiliary = run + auxiliary;
std::string const lumiAuxiliary = lumi + auxiliary;
std::string const eventAuxiliary = event + auxiliary;
const NameArray metaTreeNames{makeNameArray(metaData)};

std::string const runProductStatus = run + productStatus; // backward compatibility
std::string const lumiProductStatus = lumi + productStatus; // backward compatibility
std::string const eventProductStatus = event + productStatus; // backward compatibility
// backward compatibility
const NameArray infoNames{makeNameArray(std::string("StatusInformation"))};

std::string const runEventEntryInfo = run + branchEntryInfo; // backward compatibility
std::string const lumiEventEntryInfo = lumi + branchEntryInfo; // backward compatibility
std::string const eventEventEntryInfo = event + branchEntryInfo; // backward compatibility
const NameArray auxiliaryNames{makeNameArray(auxiliary)};

std::string const runProductProvenance = run + productProvenance;
std::string const lumiProductProvenance = lumi + productProvenance;
std::string const eventProductProvenance = event + productProvenance;
// backward compatibility
const NameArray productStatusNames{makeNameArray(productStatus)};

std::string const majorIndex = ".id_.run_";
std::string const runMajorIndex = runAuxiliary + majorIndex;
std::string const lumiMajorIndex = lumiAuxiliary + majorIndex;
std::string const eventMajorIndex = eventAuxiliary + majorIndex;
// backward compatibility
const NameArray eventEntryInfoNames{makeNameArray(branchEntryInfo)};

std::string const runMinorIndex; // empty
std::string const lumiMinorIndex = lumiAuxiliary + ".id_.luminosityBlock_";
std::string const eventMinorIndex = eventAuxiliary + ".id_.event_";
const NameArray productProvenanceNames{makeNameArray(productProvenance)};

std::string const runAux = run + aux;
std::string const lumiAux = lumi + aux;
std::string const eventAux = event + aux;
// backward compatibility
const NameArray auxNames{makeNameArray(aux)};

std::string const entryDescriptionTree = "EntryDescription";
std::string const entryDescriptionIDBranch = "Hash";
Expand Down Expand Up @@ -95,53 +88,50 @@ namespace edm {
std::string const idToParameterSetBlobsBranch = "IdToParameterSetsBlobs";
} // namespace

std::string const& BranchTypeToString(BranchType const& branchType) {
return ((branchType == InEvent) ? event : ((branchType == InRun) ? run : lumi));
}
std::string const& BranchTypeToString(BranchType const& branchType) { return branchTypeNames[branchType]; }

std::string const& BranchTypeToProductTreeName(BranchType const& branchType) {
return ((branchType == InEvent) ? events : ((branchType == InRun) ? runs : lumis));
assert(branchType < eventLumiRunSize);
return treeNames[branchType];
}

std::string const& BranchTypeToMetaDataTreeName(BranchType const& branchType) {
return ((branchType == InEvent) ? eventMeta : ((branchType == InRun) ? runMeta : lumiMeta));
assert(branchType < eventLumiRunSize);
return metaTreeNames[branchType];
}

std::string const& BranchTypeToInfoTreeName(BranchType const& branchType) { // backward compatibility
return ((branchType == InEvent) ? eventInfo
: ((branchType == InRun) ? runInfo : lumiInfo)); // backward compatibility
} // backward compatibility
// backward compatibility
std::string const& BranchTypeToInfoTreeName(BranchType const& branchType) {
assert(branchType < eventLumiRunSize);
return infoNames[branchType];
}

std::string const& BranchTypeToAuxiliaryBranchName(BranchType const& branchType) {
return ((branchType == InEvent) ? eventAuxiliary : ((branchType == InRun) ? runAuxiliary : lumiAuxiliary));
assert(branchType < eventLumiRunSize);
return auxiliaryNames[branchType];
}

std::string const& BranchTypeToAuxBranchName(BranchType const& branchType) { // backward compatibility
return ((branchType == InEvent) ? eventAux : ((branchType == InRun) ? runAux : lumiAux)); // backward compatibility
} // backward compatibility

std::string const& BranchTypeToProductStatusBranchName(BranchType const& branchType) { // backward compatibility
return ((branchType == InEvent)
? eventProductStatus
: ((branchType == InRun) ? runProductStatus : lumiProductStatus)); // backward compatibility
} // backward compatibility

std::string const& BranchTypeToBranchEntryInfoBranchName(BranchType const& branchType) {
return ((branchType == InEvent) ? eventEventEntryInfo
: ((branchType == InRun) ? runEventEntryInfo : lumiEventEntryInfo));
// backward compatibility
std::string const& BranchTypeToAuxBranchName(BranchType const& branchType) {
assert(branchType < eventLumiRunSize);
return auxNames[branchType];
}

std::string const& BranchTypeToProductProvenanceBranchName(BranchType const& branchType) {
return ((branchType == InEvent) ? eventProductProvenance
: ((branchType == InRun) ? runProductProvenance : lumiProductProvenance));
// backward compatibility
std::string const& BranchTypeToProductStatusBranchName(BranchType const& branchType) {
assert(branchType < eventLumiRunSize);
return productStatusNames[branchType];
}

std::string const& BranchTypeToMajorIndexName(BranchType const& branchType) {
return ((branchType == InEvent) ? eventMajorIndex : ((branchType == InRun) ? runMajorIndex : lumiMajorIndex));
// backward compatibility
std::string const& BranchTypeToBranchEntryInfoBranchName(BranchType const& branchType) {
assert(branchType < eventLumiRunSize);
return eventEntryInfoNames[branchType];
}

std::string const& BranchTypeToMinorIndexName(BranchType const& branchType) {
return ((branchType == InEvent) ? eventMinorIndex : ((branchType == InRun) ? runMinorIndex : lumiMinorIndex));
std::string const& BranchTypeToProductProvenanceBranchName(BranchType const& branchType) {
assert(branchType < eventLumiRunSize);
return productProvenanceNames[branchType];
}

namespace poolNames {
Expand Down Expand Up @@ -215,12 +205,12 @@ namespace edm {
// Branch on ParameterSets Tree
std::string const& idToParameterSetBlobsBranchName() { return idToParameterSetBlobsBranch; }

std::string const& eventTreeName() { return events; }
std::string const& eventTreeName() { return treeNames[InEvent]; }

std::string const& eventMetaDataTreeName() { return eventMeta; }
std::string const& eventMetaDataTreeName() { return metaTreeNames[InEvent]; }

std::string const& eventHistoryTreeName() { return eventHistory; }
std::string const& luminosityBlockTreeName() { return lumis; }
std::string const& runTreeName() { return runs; }
std::string const& luminosityBlockTreeName() { return treeNames[InLumi]; }
std::string const& runTreeName() { return treeNames[InRun]; }
} // namespace poolNames
} // namespace edm
61 changes: 21 additions & 40 deletions DataFormats/Provenance/src/ProductRegistry.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,11 @@ namespace edm {
: frozen_(false),
productProduced_(),
anyProductProduced_(false),
eventProductLookup_(new ProductResolverIndexHelper),
lumiProductLookup_(new ProductResolverIndexHelper),
runProductLookup_(new ProductResolverIndexHelper),
eventNextIndexValue_(0),
lumiNextIndexValue_(0),
runNextIndexValue_(0),

productLookups_{{std::make_unique<ProductResolverIndexHelper>(),
std::make_unique<ProductResolverIndexHelper>(),
std::make_unique<ProductResolverIndexHelper>(),
std::make_unique<ProductResolverIndexHelper>()}},
nextIndexValues_(),
branchIDToIndex_() {
for (bool& isProduced : productProduced_)
isProduced = false;
Expand All @@ -54,13 +52,10 @@ namespace edm {
anyProductProduced_ = false;

// propagate_const<T> has no reset() function
eventProductLookup_ = std::make_unique<ProductResolverIndexHelper>();
lumiProductLookup_ = std::make_unique<ProductResolverIndexHelper>();
runProductLookup_ = std::make_unique<ProductResolverIndexHelper>();

eventNextIndexValue_ = 0;
lumiNextIndexValue_ = 0;
runNextIndexValue_ = 0;
for (auto& iterProductLookup : productLookups_) {
iterProductLookup = std::make_unique<ProductResolverIndexHelper>();
}
nextIndexValues_.fill(0);

branchIDToIndex_.clear();
}
Expand Down Expand Up @@ -147,19 +142,11 @@ namespace edm {
}

std::shared_ptr<ProductResolverIndexHelper const> ProductRegistry::productLookup(BranchType branchType) const {
if (branchType == InEvent)
return transient_.eventProductLookup();
if (branchType == InLumi)
return transient_.lumiProductLookup();
return transient_.runProductLookup();
return get_underlying_safe(transient_.productLookups_[branchType]);
}

std::shared_ptr<ProductResolverIndexHelper> ProductRegistry::productLookup(BranchType branchType) {
if (branchType == InEvent)
return transient_.eventProductLookup();
if (branchType == InLumi)
return transient_.lumiProductLookup();
return transient_.runProductLookup();
return get_underlying_safe(transient_.productLookups_[branchType]);
}

void ProductRegistry::setFrozen(bool initializeLookupInfo) {
Expand Down Expand Up @@ -451,13 +438,15 @@ namespace edm {
throwMissingDictionariesException(missingDictionaries, context, producedTypes, branchNamesForMissing);
}

productLookup(InEvent)->setFrozen();
productLookup(InLumi)->setFrozen();
productLookup(InRun)->setFrozen();
for (auto& iterProductLookup : transient_.productLookups_) {
iterProductLookup->setFrozen();
}

transient_.eventNextIndexValue_ = productLookup(InEvent)->nextIndexValue();
transient_.lumiNextIndexValue_ = productLookup(InLumi)->nextIndexValue();
transient_.runNextIndexValue_ = productLookup(InRun)->nextIndexValue();
unsigned int indexIntoNextIndexValue = 0;
for (auto const& iterProductLookup : transient_.productLookups_) {
transient_.nextIndexValues_[indexIntoNextIndexValue] = iterProductLookup->nextIndexValue();
++indexIntoNextIndexValue;
}

for (auto const& product : productList_) {
auto const& desc = product.second;
Expand Down Expand Up @@ -576,18 +565,10 @@ namespace edm {
}

ProductResolverIndex const& ProductRegistry::getNextIndexValue(BranchType branchType) const {
if (branchType == InEvent)
return transient_.eventNextIndexValue_;
if (branchType == InLumi)
return transient_.lumiNextIndexValue_;
return transient_.runNextIndexValue_;
return transient_.nextIndexValues_[branchType];
}

ProductResolverIndex& ProductRegistry::nextIndexValue(BranchType branchType) {
if (branchType == InEvent)
return transient_.eventNextIndexValue_;
if (branchType == InLumi)
return transient_.lumiNextIndexValue_;
return transient_.runNextIndexValue_;
return transient_.nextIndexValues_[branchType];
}
} // namespace edm
5 changes: 3 additions & 2 deletions FWCore/Framework/interface/BranchActionType.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#define Framework_BranchActionType_h

/*----------------------------------------------------------------------

BranchActionType: BranchAction

----------------------------------------------------------------------*/
Expand All @@ -12,7 +12,8 @@ namespace edm {
BranchActionGlobalBegin = 0,
BranchActionStreamBegin = 1,
BranchActionStreamEnd = 2,
BranchActionGlobalEnd = 3
BranchActionGlobalEnd = 3,
BranchActionProcessBlockInput = 4
};
}
#endif
Loading