Project

General

Profile

Necessary Maintenance #6339

Duplication of utility functionality in art vs cetlib

Added by Ben Morgan over 6 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Infrastructure
Target version:
Start date:
05/23/2014
Due date:
% Done:

100%

Estimated time:
16.00 h
Spent time:
Scope:
Internal
Experiment:
-
SSI Package:
Duration:

Description

Art re-implements several bits of functionality that appear to already be present in cetlib, for example I've identified MD5, CRC32, CPU timing.

A single point of contact for such interfaces should be chosen to minimize duplication and confusion.


Related issues

Related to art - Necessary Maintenance #7731: Determine if art summary TimeReport can be replaced by TimeTracker summaryClosed01/30/2015

Associated revisions

Revision 3a6e9150 (diff)
Added by Kyle Knoepfel over 5 years ago

Implement issue #6339

Removed art implementations of crc32, md5 and CPUTimer; tests moved to cetlib

History

#1 Updated by Christopher Green over 6 years ago

  • Tracker changed from Bug to Necessary Maintenance
  • Category set to Infrastructure
  • Status changed from New to Feedback

With particular regard to the CRC32 and MD5 functionality, the use of the art-provided functions within art is historical. We would prefer to switch to the cetlib-provided implementations ourselves, but we have always been nervous of the data compatibility implications of switching if the implementations are not exactly compatible.

There is a wide range of opinion on this matter; if the stakeholders agree that a reasonable validation on a range of samples provides sufficient assurance of compatibility, we can certainly make the switch.

The CPU timer functionality is planned for overhaul for the version:"Sirius A" release. We will consider the reconciliation of the cetlib and art functionality in this area at that time.

#2 Updated by Christopher Green over 6 years ago

  • Target version set to 1.13.00

#3 Updated by Christopher Green almost 6 years ago

  • Status changed from Feedback to Accepted
  • Target version changed from 1.13.00 to 1.14.00

Due to the requirement for a Sirius A release on a short timescale, this issue is deferred.

#4 Updated by Christopher Green almost 6 years ago

#5 Updated by Christopher Green over 5 years ago

  • Estimated time set to 16.00 h

#6 Updated by Kyle Knoepfel over 5 years ago

  • Status changed from Accepted to Assigned
  • Assignee set to Kyle Knoepfel

#7 Updated by Kyle Knoepfel over 5 years ago

  • Status changed from Assigned to Resolved
  • % Done changed from 0 to 100

The CRC32, MD5, and CPU-timing utilities are now located solely in cetlib. This code consolidation could require interface changes for the user, as described below. Only users who use the following files will need to adjust their code:

  • cetlib/md5.h
  • art/Utilities/CPUTimer.h
  • art/Utilities/CRC32Calculator.h
  • art/Utilities/Digest.h
  • art/Utilites/md5.h

CRC32 algorithm

// Get the correct headers
-#include "art/Utilities/CRC32Calculator.h" 
+#include "cetlib/crc32.h" 

// Creating and using the object
- art::CRC32Calculator c(someString);
- auto chksum = c.checksum();
+ cet::crc32 c(someString);
+ auto chksum = c.digest(); 

MD5 algorithm

// Get the correct headers
-#include "art/Utilities/Digest.h" 
+#include "cetlib/MD5Digest.h" 

// Constructing a digest object
- art::Digest md5alg(stringRep);
+ cet::MD5Digest md5alg(stringRep);

// If you have to use the MD5Result class
- art::MD5Result res;
+ cet::MD5Result res;

// Rest of the interface is the same.

N.B. The art/Utilities/md5.h and cetlib/md5.h files have been removed in favor of cetlib/MD5Digest.h. If you explicitly use the functions/classes in either of those md5.h files, please contact the artists for guidance as to how to adjust your code. The polarssl/md5.h file remains in place.

CPU timer

// Get the correct headers
-#include "art/Utilities/CPUTimer.h" 
+#include "cetlib/cpu_timer.h" 

// Creating the timer object ...
- art::CPUTimer timer;
+ cet::cpu_timer timer;

// Rest of the interface can remain the same, or one can
// explicitly use the additional cetlib member functions.

Overhaul of the CPU timer will be considered when issue #7731 is addressed.

Implemented with commits cetlib:5c7568ef632d7d7bc8ce84bf4deeb9de30efc03d and art:3a6e9150ff6c1921b7c881172a28fffb3eb1827a.

#8 Updated by Kyle Knoepfel over 5 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF