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

SiStrip (un)packer: fixes and support for nonstandard ZS(lite) modes #23417

Merged
merged 21 commits into from Jun 18, 2018
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
a286523
Fix packet codes for ZS and ZS lite
pieterdavid Mar 21, 2018
af05a2b
Add other ZS modes to SiStripRawToDigiUnpacker
pieterdavid Mar 21, 2018
435b8a0
Fix uninitialized member in FEDBSChannelUnpacker
pieterdavid Mar 24, 2018
268fa9d
A few more FEDBSChannelUnpacker fixes
pieterdavid Mar 27, 2018
ad14f83
Add packet code option for SiStripDigiToRawModule
pieterdavid May 24, 2018
92f3b01
Fix a regression for VR10 unpacking
pieterdavid May 25, 2018
b736f01
use an InputTag in SiStripDigiToRawModule
pieterdavid May 25, 2018
60d19be
adding VR packet code cases
alesaggio May 25, 2018
493e4c7
Fix FEDBSChannelUnpacker::hasData for other sizes than 10 and 16
pieterdavid May 30, 2018
e810ba3
VR and ZR fixed
alesaggio May 31, 2018
86a9c4c
Merge branch 'SiStripPacker_fixZSmodes_102' of https://github.com/ale…
pieterdavid Jun 1, 2018
a925990
ZS packer patch: cleanup
pieterdavid Jun 1, 2018
a3ebb8a
Packer: use ZS code also for ZS lite
pieterdavid Jun 1, 2018
9ce7d97
packer: readout mode parsing and empty default packet code
pieterdavid Jun 1, 2018
8113325
also for ZS packer: push directly packet code
pieterdavid Jun 1, 2018
983db85
code checks fixes (empty string tests)
pieterdavid Jun 1, 2018
a980e34
Update configs to changes in SiStripDigiToRaw parameters
pieterdavid Jun 1, 2018
904aa8b
Use default ZS packing if the packet code is zero
pieterdavid Jun 6, 2018
57234aa
Print SiStrip unpacker warnings and exceptions once per event
pieterdavid Jun 7, 2018
06b4b3b
strip unpacker: print warnings on first occurrence, summary in endStream
pieterdavid Jun 7, 2018
81ef51b
sistrip::FEDBSChannelUnpacker: use BITS_PER_BYTE constant instead of 8
pieterdavid Jun 11, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions Configuration/StandardSequences/python/DigiToRawDM_cff.py
Expand Up @@ -5,8 +5,7 @@
# Re-define inputs to look at the DataMixer output
#
siPixelRawData.InputLabel = cms.InputTag("mixData:siPixelDigisDM")
SiStripDigiToRaw.InputModuleLabel = cms.string('mixData')
SiStripDigiToRaw.InputDigiLabel = cms.string('siStripDigisDM')
SiStripDigiToRaw.InputDigis = cms.InputTag("mixData", "siStripDigisDM")
#
ecalPacker.Label = 'DMEcalDigis'
ecalPacker.InstanceEB = 'ebDigis'
Expand Down
Expand Up @@ -6,9 +6,9 @@

import EventFilter.SiStripRawToDigi.SiStripDigiToRaw_cfi
SiStripDigiToZSRaw = EventFilter.SiStripRawToDigi.SiStripDigiToRaw_cfi.SiStripDigiToRaw.clone(
InputModuleLabel = 'siStripZeroSuppression',
InputDigiLabel = cms.string('VirginRaw'),
InputDigis = cms.InputTag('siStripZeroSuppression', 'VirginRaw'),
FedReadoutMode = cms.string('ZERO_SUPPRESSED'),
PacketCode = cms.string('ZERO_SUPPRESSED'),
CopyBufferHeader = cms.bool(True),
RawDataTag = cms.InputTag('rawDataCollector')
)
Expand Down
21 changes: 16 additions & 5 deletions EventFilter/SiStripRawToDigi/interface/SiStripFEDBuffer.h
Expand Up @@ -158,7 +158,6 @@ namespace sistrip {
const uint8_t* data_;
uint16_t oldWordOffset_;
uint16_t currentWordOffset_;
uint16_t currentBitOffset_;
uint16_t currentLocalBitOffset_;
uint16_t bitOffsetIncrement_;
uint8_t currentStrip_;
Expand Down Expand Up @@ -209,7 +208,7 @@ namespace sistrip {
inline FEDBSChannelUnpacker::FEDBSChannelUnpacker()
: data_(nullptr),
oldWordOffset_(0), currentWordOffset_(0),
currentBitOffset_(0), currentLocalBitOffset_(0),
currentLocalBitOffset_(0),
bitOffsetIncrement_(10),
currentStrip_(0),
channelPayloadOffset_(0), channelPayloadLength_(0),
Expand All @@ -219,13 +218,15 @@ namespace sistrip {
inline FEDBSChannelUnpacker::FEDBSChannelUnpacker(const uint8_t* payload, const uint16_t channelPayloadOffset, const int16_t channelPayloadLength, const uint16_t offsetIncrement, bool useZS)
: data_(payload),
oldWordOffset_(0), currentWordOffset_(channelPayloadOffset),
currentBitOffset_(0), currentLocalBitOffset_(0),
currentLocalBitOffset_(0),
bitOffsetIncrement_(offsetIncrement),
currentStrip_(0),
channelPayloadOffset_(channelPayloadOffset),
channelPayloadLength_(channelPayloadLength),
useZS_(useZS), valuesLeftInCluster_(0)
{
if (bitOffsetIncrement_>16) throwBadWordLength(bitOffsetIncrement_); // more than 2 words... still to be implemented
if (useZS_ && channelPayloadLength_) readNewClusterInfo();
}

inline FEDBSChannelUnpacker FEDBSChannelUnpacker::virginRawModeUnpacker(const FEDChannel& channel, uint16_t num_bits)
Expand Down Expand Up @@ -270,13 +271,19 @@ namespace sistrip {

inline bool FEDBSChannelUnpacker::hasData() const
{
return (currentWordOffset_<channelPayloadOffset_+channelPayloadLength_);
const uint16_t nextChanWordOffset = channelPayloadOffset_+channelPayloadLength_;
if ( currentWordOffset_ + 1 < nextChanWordOffset ) {
return true; // fast case: 2 bytes always fit an ADC (even if offset)
} else { // close to end
const uint16_t plusOneBitOffset = currentLocalBitOffset_+bitOffsetIncrement_;
const uint16_t plusOneWordOffset = currentWordOffset_ + plusOneBitOffset/8;
Copy link
Contributor

Choose a reason for hiding this comment

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

This magic number "8" appears rather often in this class.
It could be more conveniently defined somewhere, perhaps as a sizeof() a given data word

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 this context it is the number of bits in a byte of raw data (uint8_t), so sizeof will not work (it gives the size of a type in bytes). I could define a constant, e.g. constexpr uint16_t BITS_PER_BYTE = 8;, but it would be only for readability.

Copy link
Contributor

Choose a reason for hiding this comment

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

That would help, indeed

return ( plusOneBitOffset % 8 ) ? ( plusOneWordOffset < nextChanWordOffset ) : ( plusOneWordOffset <= nextChanWordOffset );
}
}

inline FEDBSChannelUnpacker& FEDBSChannelUnpacker::operator ++ ()
{
oldWordOffset_ = currentWordOffset_;
currentBitOffset_ += bitOffsetIncrement_;
currentLocalBitOffset_ += bitOffsetIncrement_;
while (currentLocalBitOffset_>=8) {
currentWordOffset_++;
Expand All @@ -302,6 +309,10 @@ namespace sistrip {

inline void FEDBSChannelUnpacker::readNewClusterInfo()
{
if ( currentLocalBitOffset_ ) {
++currentWordOffset_;
currentLocalBitOffset_ = 0;
}
currentStrip_ = data_[(currentWordOffset_++)^7];
valuesLeftInCluster_ = data_[(currentWordOffset_++)^7]-1;
}
Expand Down
Expand Up @@ -79,10 +79,9 @@ namespace sistrip {
static const uint8_t PACKET_CODE_PROC_RAW8_BOTBOT = 0xCA;
static const uint8_t PACKET_CODE_PROC_RAW8_TOPBOT = 0xB2;
static const uint8_t PACKET_CODE_ZERO_SUPPRESSED = 0xEA;
static const uint8_t PACKET_CODE_ZERO_SUPPRESSED_LITE10 = 0x8A;
static const uint8_t PACKET_CODE_ZERO_SUPPRESSED_LITE8 = 0xEA;
static const uint8_t PACKET_CODE_ZERO_SUPPRESSED_LITE8_BOTBOT = 0xCA;
static const uint8_t PACKET_CODE_ZERO_SUPPRESSED_LITE8_TOPBOT = 0xAA;
static const uint8_t PACKET_CODE_ZERO_SUPPRESSED10 = 0x8A;
static const uint8_t PACKET_CODE_ZERO_SUPPRESSED8_BOTBOT = 0xCA;
static const uint8_t PACKET_CODE_ZERO_SUPPRESSED8_TOPBOT = 0xAA;

//enum values are values which appear in buffer. DO NOT CHANGE!
//see http://cmsdoc.cern.ch/cms/TRIDAS/horizontal/RUWG/DAQ_IF_guide/DAQ_IF_guide.html
Expand Down Expand Up @@ -151,6 +150,7 @@ namespace sistrip {
FEDBufferFormat fedBufferFormatFromString(const std::string& bufferFormatString);
FEDHeaderType fedHeaderTypeFromString(const std::string& headerTypeString);
FEDReadoutMode fedReadoutModeFromString(const std::string& readoutModeString);
uint8_t packetCodeFromString(const std::string& packetCodeString, FEDReadoutMode mode);
FEDDAQEventType fedDAQEventTypeFromString(const std::string& daqEventTypeString);

//
Expand Down Expand Up @@ -1492,16 +1492,13 @@ namespace sistrip {
case READOUT_MODE_PROC_RAW:
return PACKET_CODE_PROC_RAW;
case READOUT_MODE_ZERO_SUPPRESSED:
return PACKET_CODE_ZERO_SUPPRESSED;
return channel(internalFEDChannelNum).packetCode();
case READOUT_MODE_ZERO_SUPPRESSED_LITE10:
case READOUT_MODE_ZERO_SUPPRESSED_LITE10_CMOVERRIDE:
return PACKET_CODE_ZERO_SUPPRESSED_LITE10;
case READOUT_MODE_ZERO_SUPPRESSED_LITE8_BOTBOT:
case READOUT_MODE_ZERO_SUPPRESSED_LITE8_BOTBOT_CMOVERRIDE:
return PACKET_CODE_ZERO_SUPPRESSED_LITE8_BOTBOT;
case READOUT_MODE_ZERO_SUPPRESSED_LITE8_TOPBOT:
case READOUT_MODE_ZERO_SUPPRESSED_LITE8_TOPBOT_CMOVERRIDE:
return PACKET_CODE_ZERO_SUPPRESSED_LITE8_TOPBOT;
case READOUT_MODE_ZERO_SUPPRESSED_LITE8:
case READOUT_MODE_ZERO_SUPPRESSED_LITE8_CMOVERRIDE:
case READOUT_MODE_PREMIX_RAW:
Expand Down
40 changes: 12 additions & 28 deletions EventFilter/SiStripRawToDigi/interface/SiStripFEDBufferGenerator.h
Expand Up @@ -32,7 +32,7 @@ namespace sistrip {
//get the 10bit value to be used for raw modes
uint16_t getSample(const uint16_t sampleNumber) const;
//get the 8 bit value to be used for ZS modes, converting it as the FED does if specified in constructor
uint8_t get8BitSample(const uint16_t sampleNumber, const FEDReadoutMode mode) const;
uint8_t get8BitSample(const uint16_t sampleNumber, uint16_t nBotBitsToDrop) const;
uint16_t get10BitSample(const uint16_t sampleNumber) const;
void setSample(const uint16_t sampleNumber, const uint16_t adcValue);
//setting value directly is equivalent to get and set Sample but without length check
Expand Down Expand Up @@ -82,21 +82,21 @@ namespace sistrip {
//If a channel is disabled then it is considered to have all zeros in the data but will be present in the data
FEDBufferPayloadCreator(const std::vector<bool>& enabledFEUnits, const std::vector<bool>& enabledChannels);
//create the payload for a particular mode
FEDBufferPayload createPayload(const FEDReadoutMode mode, const FEDStripData& data) const;
FEDBufferPayload operator () (const FEDReadoutMode mode, const FEDStripData& data) const;
FEDBufferPayload createPayload(FEDReadoutMode mode, uint8_t packetCode, const FEDStripData& data) const;
FEDBufferPayload operator () (FEDReadoutMode mode, uint8_t packetCode, const FEDStripData& data) const;
private:
//fill vector with channel data
void fillChannelBuffer(std::vector<uint8_t>* channelBuffer, const FEDReadoutMode mode,
void fillChannelBuffer(std::vector<uint8_t>* channelBuffer, FEDReadoutMode mode, uint8_t packetCode,
const FEDStripData::ChannelData& data, const bool channelEnabled) const;
//fill the vector with channel data for raw mode
void fillRawChannelBuffer(std::vector<uint8_t>* channelBuffer, const uint8_t packetCode,
const FEDStripData::ChannelData& data, const bool channelEnabled, const bool reorderData) const;
//fill the vector with channel data for zero suppressed modes
void fillZeroSuppressedChannelBuffer(std::vector<uint8_t>* channelBuffer, const FEDStripData::ChannelData& data, const bool channelEnabled) const;
void fillZeroSuppressedChannelBuffer(std::vector<uint8_t>* channelBuffer, const uint8_t packetCode, const FEDStripData::ChannelData& data, const bool channelEnabled) const;
void fillZeroSuppressedLiteChannelBuffer(std::vector<uint8_t>* channelBuffer, const FEDStripData::ChannelData& data, const bool channelEnabled, const FEDReadoutMode mode) const;
void fillPreMixRawChannelBuffer(std::vector<uint8_t>* channelBuffer, const FEDStripData::ChannelData& data, const bool channelEnabled) const;
//add the ZS cluster data for the channel to the end of the vector
void fillClusterData(std::vector<uint8_t>* channelBuffer, const FEDStripData::ChannelData& data, const FEDReadoutMode mode) const;
void fillClusterData(std::vector<uint8_t>* channelBuffer, uint8_t packetCode, const FEDStripData::ChannelData& data, const FEDReadoutMode mode) const;
void fillClusterDataPreMixMode(std::vector<uint8_t>* channelBuffer, const FEDStripData::ChannelData& data) const;
std::vector<bool> feUnitsEnabled_;
std::vector<bool> channelsEnabled_;
Expand Down Expand Up @@ -146,7 +146,8 @@ namespace sistrip {
//FEDRawData object will be resized to fit buffer and filled
void generateBuffer(FEDRawData* rawDataObject,
const FEDStripData& data,
const uint16_t sourceID) const;
uint16_t sourceID,
uint8_t packetCode) const;
private:
//method to fill buffer at pointer from pre generated components (only the length and CRC will be changed)
//at least bufferSizeInBytes(feHeader,payload) must have already been allocated
Expand Down Expand Up @@ -231,26 +232,9 @@ namespace sistrip {
return data_[sampleNumber];
}

inline uint8_t FEDStripData::ChannelData::get8BitSample(const uint16_t sampleNumber, const FEDReadoutMode mode) const
inline uint8_t FEDStripData::ChannelData::get8BitSample(const uint16_t sampleNumber, uint16_t nBotBitsToDrop) const
{
uint16_t sample = getSample(sampleNumber);
// one start shifting the word
switch (mode) {
case READOUT_MODE_ZERO_SUPPRESSED:
case READOUT_MODE_ZERO_SUPPRESSED_LITE8:
case READOUT_MODE_ZERO_SUPPRESSED_LITE8_CMOVERRIDE:
break;
case READOUT_MODE_ZERO_SUPPRESSED_LITE8_TOPBOT:
case READOUT_MODE_ZERO_SUPPRESSED_LITE8_TOPBOT_CMOVERRIDE:
sample = (sample>>1);
break;
case READOUT_MODE_ZERO_SUPPRESSED_LITE8_BOTBOT:
case READOUT_MODE_ZERO_SUPPRESSED_LITE8_BOTBOT_CMOVERRIDE:
sample = (sample>>2);
break;
default:
throw cms::Exception("FEDBufferGenerator") << "Invalid readout mode requested for 8-bit sample retrieval";
}
uint16_t sample = getSample(sampleNumber) >> nBotBitsToDrop;
if (dataIs8Bit_) {
return (0xFF & sample);
}
Expand Down Expand Up @@ -309,9 +293,9 @@ namespace sistrip {
channelsEnabled_(channelsEnabled)
{}

inline FEDBufferPayload FEDBufferPayloadCreator::operator () (const FEDReadoutMode mode, const FEDStripData& data) const
inline FEDBufferPayload FEDBufferPayloadCreator::operator () (FEDReadoutMode mode, uint8_t packetCode, const FEDStripData& data) const
{
return createPayload(mode,data);
return createPayload(mode, packetCode, data);
}

//FEDBufferGenerator
Expand Down
10 changes: 6 additions & 4 deletions EventFilter/SiStripRawToDigi/plugins/SiStripDigiToRaw.cc
Expand Up @@ -19,9 +19,11 @@ namespace sistrip {

// -----------------------------------------------------------------------------
/** */
DigiToRaw::DigiToRaw( FEDReadoutMode mode,
bool useFedKey ) :
DigiToRaw::DigiToRaw( FEDReadoutMode mode,
uint8_t packetCode,
bool useFedKey ) :
mode_(mode),
packetCode_(packetCode),
useFedKey_(useFedKey),
bufferGenerator_()
{
Expand Down Expand Up @@ -299,7 +301,7 @@ namespace sistrip {
}
//create the buffer
FEDRawData& fedrawdata = buffers->FEDData( *ifed );
bufferGenerator_.generateBuffer(&fedrawdata,fedData,*ifed);
bufferGenerator_.generateBuffer(&fedrawdata, fedData, *ifed, packetCode_);

if ( edm::isDebugEnabled() ) {
std::ostringstream debugStream;
Expand Down Expand Up @@ -410,7 +412,7 @@ namespace sistrip {
}
//create the buffer
FEDRawData& fedrawdata = buffers->FEDData( *ifed );
bufferGenerator_.generateBuffer(&fedrawdata,fedData,*ifed);
bufferGenerator_.generateBuffer(&fedrawdata, fedData, *ifed, packetCode_);

if ( edm::isDebugEnabled() ) {
std::ostringstream debugStream;
Expand Down
3 changes: 2 additions & 1 deletion EventFilter/SiStripRawToDigi/plugins/SiStripDigiToRaw.h
Expand Up @@ -29,7 +29,7 @@ namespace sistrip {

public: // ----- public interface -----

DigiToRaw( FEDReadoutMode, bool use_fed_key );
DigiToRaw(FEDReadoutMode mode, uint8_t packetCode, bool use_fed_key);
~DigiToRaw();

//digi to raw with default headers
Expand Down Expand Up @@ -79,6 +79,7 @@ namespace sistrip {
uint16_t STRIP(const edm::DetSet<SiStripRawDigi>::const_iterator& it, const edm::DetSet<SiStripRawDigi>::const_iterator& begin) const;

FEDReadoutMode mode_;
uint8_t packetCode_;
bool useFedKey_;
FEDBufferGenerator bufferGenerator_;

Expand Down
14 changes: 7 additions & 7 deletions EventFilter/SiStripRawToDigi/plugins/SiStripDigiToRawModule.cc
Expand Up @@ -20,12 +20,12 @@ namespace sistrip {
//fill Descriptions needed to define default parameters
void DigiToRawModule::fillDescriptions(edm::ConfigurationDescriptions & descriptions) {
edm::ParameterSetDescription desc;
desc.add<std::string>("InputModuleLabel", "simSiStripDigis");
desc.add<std::string>("InputDigiLabel", "ZeroSuppressed");
desc.add<std::string>("FedReadoutMode", "ZERO_SUPPRESSED");
desc.add<std::string>("PacketCode", "ZERO_SUPPRESSED");
desc.add<bool>("UseFedKey", false);
desc.add<bool>("UseWrongDigiType", false);
desc.add<bool>("CopyBufferHeader", false);
desc.add<edm::InputTag>("InputDigis", edm::InputTag("simSiStripDigis", "ZeroSuppressed"));
desc.add<edm::InputTag>("RawDataTag", edm::InputTag("rawDataCollector"));
descriptions.add("SiStripDigiToRawModule",desc);
}
Expand All @@ -35,13 +35,13 @@ namespace sistrip {
Creates instance of DigiToRaw converter, defines EDProduct type.
*/
DigiToRawModule::DigiToRawModule( const edm::ParameterSet& pset ) :
inputModuleLabel_( pset.getParameter<std::string>( "InputModuleLabel" ) ),
inputDigiLabel_( pset.getParameter<std::string>( "InputDigiLabel" ) ),
copyBufferHeader_(pset.getParameter<bool>("CopyBufferHeader")),
mode_( fedReadoutModeFromString(pset.getParameter<std::string>( "FedReadoutMode" ))),
packetCode_(packetCodeFromString(pset.getParameter<std::string>("PacketCode"), mode_)),
rawdigi_( false ),
digiToRaw_(nullptr),
eventCounter_(0),
inputDigiTag_(pset.getParameter<edm::InputTag>("InputDigis")),
rawDataTag_(pset.getParameter<edm::InputTag>("RawDataTag"))
{
if ( edm::isDebugEnabled() ) {
Expand Down Expand Up @@ -89,12 +89,12 @@ namespace sistrip {
}

// Create instance of DigiToRaw formatter
digiToRaw_ = new DigiToRaw( mode_, pset.getParameter<bool>("UseFedKey") );
digiToRaw_ = new DigiToRaw(mode_, packetCode_, pset.getParameter<bool>("UseFedKey"));

if (rawdigi_) {
tokenRawDigi = consumes< edm::DetSetVector<SiStripRawDigi> >(edm::InputTag(inputModuleLabel_, inputDigiLabel_));
tokenRawDigi = consumes< edm::DetSetVector<SiStripRawDigi> >(inputDigiTag_);
} else {
tokenDigi = consumes< edm::DetSetVector<SiStripDigi> >(edm::InputTag(inputModuleLabel_, inputDigiLabel_));
tokenDigi = consumes< edm::DetSetVector<SiStripDigi> >(inputDigiTag_);
}
if (copyBufferHeader_){
//CAMM input raw module label or same as digi ????
Expand Down
4 changes: 2 additions & 2 deletions EventFilter/SiStripRawToDigi/plugins/SiStripDigiToRawModule.h
Expand Up @@ -40,14 +40,14 @@ namespace sistrip {

private:

std::string inputModuleLabel_;
std::string inputDigiLabel_;
//CAMM can we do without this bool based on the mode ?
bool copyBufferHeader_;
FEDReadoutMode mode_;
uint8_t packetCode_;
bool rawdigi_;
DigiToRaw* digiToRaw_;
uint32_t eventCounter_;
edm::InputTag inputDigiTag_;
edm::EDGetTokenT< edm::DetSetVector<SiStripRawDigi> > tokenRawDigi;
edm::EDGetTokenT< edm::DetSetVector<SiStripDigi> > tokenDigi;
edm::InputTag rawDataTag_;
Expand Down
Expand Up @@ -163,5 +163,9 @@ namespace sistrip {
}
}

void RawToDigiModule::endStream()
{
rawToDigi_->printWarningSummary();
}
}

Expand Up @@ -32,6 +32,7 @@ namespace sistrip {

void beginRun( const edm::Run&, const edm::EventSetup& ) override;
void produce( edm::Event&, const edm::EventSetup& ) override;
void endStream() override;

private:

Expand Down