EMTF March 2020 emulator update #29262

jiafulow · 2020-03-21T18:14:53Z

PR description:

This PR adds/updates the interface to all the Phase-2 trigger primitives. This is the first PR from a series of EMTF emulator updates planned for 2020.

The EMTF emulator needs to be upgraded to handle all the Phase-2 trigger primitives, including GEM, ME0, and iRPC. It's also nice to be able to handle DT/TwinMux. Majority of the work has been done during the development for Phase-2 EMTF++, and used for L1T Phase-2 Upgrade TDR studies. These changes are now being ported to the official CMSSW. For Run 3 specifically, the GE1/1 interface is needed to continue the rest of the EMTF-GEM integration.

There are a lot of changes because what's in the existing EMTF emulator related to GEM/ME0/iRPC is not correct/updated. Also, unused codes related to converting track trigger stubs into muon trigger primitives have been removed.

Notes:

The definition of Subsector() for EMTFHit converted from RPC hit has changed. It is now consistent with the CSC definition (1 or 2 in ME1, 0 in ME2,3,4). The old definition is no longer stored, but can be recovered via Subsector_RPC().
GEM input is now GEMPadDigiClusterCollection.
ME0 input is now ME0TriggerDigiCollection.
Regarding input: access to simTwinMuxDigis, simDtTriggerPrimitiveDigis, rpcRecHits, me0TriggerConvertedPseudoDigis has been added, but none of them are turned on by default.
Regarding output: no change. While the interface has been implemented, the rest of the track finder logic has not been updated to use the new trigger primitives.

Additional notes:

No dependence on other PRs or externals.
- This PR does not interfere with the recent EMTF PRs by Andrew that are related to UL2016 emulation bugs.
The plan for EMTF emulator update is discussed here.

PR validation:

From runTheMatrix.py -l limited -i all -j10:

18 17 16 10 4 1 1 1 1 tests passed, 16 0 0 0 0 0 0 0 0 failed
(all the errors are due to DAS ERROR - not sure what's wrong?)

From runTheMatrix.py -w upgrade -l 20434.0,20504.0 -j10:

2 2 2 2 tests passed, 0 0 0 0 failed

Considering this is a big PR, there might be bugs. But we will try to detect and fix them as soon as possible.

Notifications: @abrinke1 @eyigitba @rekovic

cmsbuild · 2020-03-21T18:15:30Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-03-21T18:23:28Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29262/14307

This PR adds an extra 184KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File L1Trigger/L1TMuon/src/TTMuonTriggerPrimitive.cc modified in PR(s): Fixed YE3 inaccuracy for Scenario 2021 #29261
- File L1Trigger/L1TMuonEndCap/src/TrackFinder.cc modified in PR(s): EMTF configures from Global Tag conditions in MC as well as data #29260

rekovic · 2020-04-17T05:24:27Z

Hi @silviodonato @kpedro88
This PR is Phase-1 oriented as it concerns the current Phase-1 EMTF emulator. It is not the Phase-2 EMTF hardware emulator which will eventually come as a separate package. This is the adjustment to the current Phase-1 emulator to be able to read input TPs from some Phase-2 detectors, one of them being GEMs which are going to be used in Run-3. This is only a preparation step needed to adjust the current emulator be able to consume GEMs for Run-3. It will remain backward compatible to Run-2 data and MC.

kpedro88 · 2020-04-17T13:31:54Z

@rekovic that doesn't seem quite in line with the PR description, which mentions Phase 2 and TDR studies several times

rekovic · 2020-04-17T15:03:01Z

@kpedro88
This package L1Trigger/L1TMuonEndCap will remain there and L1TMuonEndCapTrackProduceris to serve for emulation of the Muon Track Finder algorithm running in Phase-1 hardware for Run2. We will use the same hardware in Run3 and this producer will be adjusted for the consumption of GEMs, but will however use the same type of pattern recognition algorithm.

The new hardware used in Phase-2 will support a different algorithm which will be emulated by a different code (not yet ready for integration). I would like to keep the two separate and avoid possibilities of disrupting development of Phase-2 or endangering backward compatibilities.

What is written in this PR description only has to do with the interface, namely ability to read the TPs.
Till the Phase-2 algorithm is emulation is integrated, the Phase-1 producer of standalone muon-tracks will be used for building of Phase2 correlated object (TrackerTrack + Muon).

This was discussed with @jiafulow.

jiafulow · 2020-04-17T16:44:51Z

@kpedro88
I'm used to call these "Phase-2" trigger primitives, although GE1/1 has been installed during this shutdown and GE2/1 will be installed before Phase-2. So, the GEM TPs are more like Phase-1.5. In any case, as Vladimir explained, this PR allows the EMTF to receive the new TPs (GEM, iRPC, ME0), but the rest of the algorithm has not been updated to use them yet. That will happen in the future PRs, and development for the Phase-2 algorithm will be kept separated from L1Trigger/L1TMuonEndCap to avoid confusion.

kpedro88 · 2020-04-17T16:48:02Z

DataFormats/L1TMuon/interface/EMTFHit.h

-#include "DataFormats/RPCDigi/interface/RPCDigi.h"
-#include "DataFormats/GEMDigi/interface/GEMPadDigi.h"
-#include "DataFormats/GEMDigi/interface/ME0PadDigi.h"
+//#include "DataFormats/RPCDigi/interface/RPCDigi.h"


delete commented-out code

kpedro88 · 2020-04-17T16:48:15Z

DataFormats/L1TMuon/interface/EMTFHit.h

    RPCDetId CreateRPCDetId() const;
+    // void ImportGEMDetId (const GEMDetId& _detId);


delete commented-out code

kpedro88 · 2020-04-17T16:48:34Z

DataFormats/L1TMuon/interface/EMTFHit.h

-    int Subsystem() const { return subsystem; }
+
+    // See L1Trigger/L1TMuon/interface/MuonTriggerPrimitive.h
+    bool Is_DT() const { return subsystem == 0; }


seems like a good case for an enum

kpedro88 · 2020-04-17T16:48:51Z

DataFormats/L1TMuon/interface/EMTFTrack.h

-    void set_RoadIdx(unsigned int bits) { _RoadIdx = bits; }
-    EMTFRoad Road() const { return _Road; }
-    unsigned int RoadIdx() const { return _RoadIdx; }
+    //void set_Road(const EMTFRoad& bits) { _Road = bits; }


delete commented-out code

kpedro88 · 2020-04-17T16:49:12Z

EventFilter/L1TRawToDigi/plugins/implementations_stage2/EMTFBlockRPC.cc

@@ -177,15 +177,22 @@ namespace l1t {
                             RPC_.Word(),
                             RPC_.Link());

+        // Rotate by 20 deg to match RPC convention in CMSSW
+        int _sector_rpc = (_subsector < 5) ? _sector : (_sector % 6) + 1;


magic numbers should be named constants (everywhere)

kpedro88 · 2020-04-17T17:01:43Z

L1Trigger/L1TMuonEndCap/src/PrimitiveSelection.cc

+    int tp_endcap = (tp_region == -1) ? 2 : tp_region;
+    int tp_station = tp_detId.station();
+    int tp_ring = 1;  // tp_detId.ring() does not exist
+    //int tp_roll      = tp_detId.roll();


delete commented-out code

kpedro88 · 2020-04-17T17:02:16Z

L1Trigger/L1TMuonEndCap/src/PrimitiveSelection.cc

+  bool sub5deg = false;
+  if (includeNeighbor_) {
+    if ((endcap_ == tp_endcap) && (get_other_neighbor(sector_) == tp_sector)) {
+      if (tp_csc_ID == 1 && tp_endcap == 1 && tp_pad >= (767 - 192)) {  // higher 1/4 of chamber


magic numbers should be named constants

kpedro88 · 2020-04-17T17:02:28Z

L1Trigger/L1TMuonEndCap/src/PrimitiveSelection.cc

+
+    int tp_bx = tp_data.bx;
+    int tp_phi = tp_data.radialAngle;
+    //int tp_phiB      = tp_data.bendingAngle;


delete commented-out code

kpedro88 · 2020-04-17T17:03:19Z

L1Trigger/L1TMuonEndCap/src/TrackFinder.cc

-    collector.extractPrimitives(GEMTag(), iEvent, tokenGEM_, muon_primitives);
+  if (useCSC_) {
+    collector.extractPrimitives(emtf::CSCTag(), &geometry_translator_, iEvent, tokenCSC_, muon_primitives);
+    //collector.extractPrimitives(emtf::CSCTag(), &geometry_translator_, iEvent, tokenCSC_, tokenCSCComparator_, muon_primitives);


delete commented-out code

kpedro88 · 2020-04-17T17:03:41Z

L1Trigger/L1TMuonEndCap/src/TrackTools.cc

+  int get_trigger_sector(int ring, int station, int chamber) {
+    int result = 0;
+    if (station > 1 && ring > 1) {
+      result = ((static_cast<unsigned>(chamber - 3) & 0x7f) / 6) + 1;  // ch 3-8->1, 9-14->2, ... 1,2 -> 6


magic numbers should be named constants

kpedro88 · 2020-04-17T17:04:51Z

@rekovic @jiafulow some of the code rule/style issues in my review seem preexisting/beyond the scope of this PR, so they can be addressed in a subsequent PR. However, I identified at least one bug and some other potential issues with how the PR will work in production, so please take a look.

rekovic · 2020-04-17T18:20:51Z

Thanks @kpedro88 .
Commented-out code can be removed indeed even now.

Thanks for the catch of a wrong size array
const char subsystem_names[][4] = {"DT", "CSC", "RPC", "GEM", "ME0"};

Some magic numbers should indeed be turned into named constants. In some cases I would say there might not be much point in doing so.

kpedro88 · 2020-04-17T18:37:35Z

@rekovic there are several reasons to avoid magic numbers in almost all circumstances. using named constants not only makes the code more readable and understandable for future maintainers by labeling numbers by their meaning, but also it enforces consistency. if a number is changed in one place, should the same number be changed somewhere else? how do i know if that 2 is the same as this other 2? this is always a future problem, not a present one, which is why the policy should always be followed whether or not a problem is "foreseen".

jiafulow · 2020-04-17T19:12:34Z

Regarding const char subsystem_names[][4] = {"DT", "CSC", "RPC", "GEM", "ME0"};
In this case, the compiler expands [][4] to [5][4], which is the necessary size for 5 elements that have maximum 3 characters plus 1 `\0' character. It is absolutely correct.

I have no idea that commented out codes are against the CMS rules. Sometimes, codes are commented out for now, but might need to be uncommented in the future. Sometimes codes are commented out to leave as a trace, so that if bugs are found in the future, it's easier to debug by checking what the previous version was. Deleting them just makes life harder for the few developers who have to work with the codes. The codes are almost always work in progress, rather than a finished product. It sounds crazy to me that this is instituted as a rule.

Regarding the magic numbers, most of the cryptic ones are already explained by the comments next to them. The trivial ones are clear from the context. There's no reason to assume any future human maintainers wouldn't be able to figure that out.

abrinke1 · 2020-04-17T19:14:23Z

Hi @kpedro88 ,

I'm all for clarity, but surely when the "magic" number is just the ID number, you don't increase the clarity by creating a new named constant? For example, in the code above "tp_csc_ID == 1" clearly indicates that the CSC ID must equal 1. This would not be made any clearer by defining some new constant:
int CSC_ID_1 = 1;
and then changing the condition to be:
tp_csc_ID == CSC_ID_1;
In fact, it would make the code more confusing. There's no "translation" of CSC ID = 1; the CSC chamber with ID = 1 is just the CSC chamber with ID = 1. I suppose you could add a geometrical description ("the chamber in the bottom left of each sector"), but that would be incredibly tedious.

Cheers,
@abrinke1

kpedro88 · 2020-04-17T19:24:47Z

@jiafulow

I have no idea that commented out codes are against the CMS rules. Sometimes, codes are commented out for now, but might need to be uncommented in the future. Sometimes codes are commented out to leave as a trace, so that if bugs are found in the future, it's easier to debug by checking what the previous version was. Deleting them just makes life harder for the few developers who have to work with the codes. The codes are almost always work in progress, rather than a finished product. It sounds crazy to me that this is instituted as a rule.

If there is a particular reason that a commented-out line of code should be kept, this should be indicated in a separate comment above that line of code. Typically, this is a temporary situation, and therefore the conditions for uncommenting that line of code should be clearly stated.

Otherwise, if a line of code is no longer needed, it is still preserved in the git history (and previous releases) for anyone who needs to look back at it.

CMSSW has 100s of contributors, with people frequently entering and leaving the collaboration. Leaving a file littered with commented-out lines of code makes it significantly harder to tell what the code is supposed to do. Once code is in the official repository, maintaining it is everyone's responsibility, not just the original developer's. Therefore, a higher standard must be met.

@jiafulow and @abrinke1:

Regarding the magic numbers, most of the cryptic ones are already explained by the comments next to them. The trivial ones are clear from the context. There's no reason to assume any future human maintainers wouldn't be able to figure that out.

Making human maintainers "figure things out" that computers can do automatically is bad practice.

In fact, it would make the code more confusing. There's no "translation" of CSC ID = 1; the CSC chamber with ID = 1 is just the CSC chamber with ID = 1. I suppose you could add a geometrical description ("the chamber in the bottom left of each sector"), but that would be incredibly tedious.

What if, for some reason, a future muon subdetector geometry swaps what is currently considered chamber 1 with chamber 2? Do you want to go through thousands of lines of code and figure out which "1"s need to be swapped with which "2"s? This is a somewhat contrived example, but there are plenty of non-contrived examples just in this PR.

abrinke1 · 2020-04-17T19:33:41Z

"What if, for some reason, a future muon subdetector geometry swaps what is currently considered chamber 1 with chamber 2? Do you want to go through thousands of lines of code and figure out which "1"s need to be swapped with which "2"s?"

My answer would be: "absolutely, yes." Because there is no central CMSSW object linking physical chambers to their numerical IDs, and if we decided to create one now, we'd have to go through all the existing L1T and CSC code and update it to use this chamber-map object. If we don't create such an object, then any change to the mapping means we have to go through all the code and find all of the new constants we created and change them by hand anyway. It saves 0 time, and adds 0 clarity compared to just using the ID number.

silviodonato · 2020-04-20T16:08:35Z

In https://cms-sw.github.io/cms_coding_rules.html you can find the "CMSSW rules".
About magic numbers, it is a generic C++ code style rules. In https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#enum1-prefer-enumerations-over-macros
you can find a trivial example

constexpr int first_month = 1;
constexpr int last_month = 12;
for (int m = first_month; m <= last_month; ++m) 
    cout << month[m] << '\n';

is more understandable than

for (int m = 1; m <= 12; ++m)
    cout << month[m] << '\n';

The code has exactly the same performance (last_month is replaced with 12 at compile time).
Therefore, we suggest to use constexpr constants instead of numbers and comments.
Clearly, this is not a huge problem, but we should try to improve the code quality of CMSSW.

About

const bool is_irpc = (tp_station == 3 || tp_station == 4) && (tp_ring == 1);

I think it is rather clear, but the following code is more understandable for people who are not L1T experts

constexpr auto RE3_1 =  std::pair<int, int>(3, 1) //station, ring
constexpr auto RE4_1 =  std::pair<int, int>(4, 1) //station, ring
[...]
const auto module = std::pair<int, int>(tp_station, tp_ring)
const bool is_irpc = ( module == RE3_1 ) ||( module == RE4_1 )

Anyhow, you can keep the 1-line version, if you strongly prefer it.

About

result = ((static_cast<unsigned>(chamber - 3) & 0x7f) / 6) + 1;  // ch 3-8->1, 9-14->2, ... 1,2 -> 6

you definitely need to use constexpr variable for 0x7fand all other numbers.

Regarding the commented code, I confirm the policy of deleting commented lines.
We can retrieve any version of the old code from GitHub.
Commented line should be used only for temporary changes.
You are right, this rule does not appear in "CMS Naming, Coding, And Style Rules". We are going to update them.

jiafulow · 2020-04-20T22:43:36Z

Let me be clear. No one in our group who is responsible for developing/maintaining this particular package would have trouble understanding

for (int m = 1; m <= 12; ++m)
    cout << month[m] << '\n';

or

const bool is_irpc = (tp_station == 3 || tp_station == 4) && (tp_ring == 1);

The hypothetical scenario where a maintainer would struggle to understand them doesn't exist. Not now, not in the future.

I'm sorry I simply don't agree that the following code is better:

constexpr auto RE3_1 =  std::pair<int, int>(3, 1) //station, ring
constexpr auto RE4_1 =  std::pair<int, int>(4, 1) //station, ring
[...]
const auto module = std::pair<int, int>(tp_station, tp_ring)
const bool is_irpc = ( module == RE3_1 ) ||( module == RE4_1 )

It's introducing throwaway variables, and it's harder to read than a one-liner. Everyone has different opinions, I've stated my opinions, and I'm not going to argue further on what is 'better' or higher code quality. In the end, our group is the one that takes responsibilities of the output of this particular package. As the developer, I have my own opinions and I am not going to be force-fed bullshit opinions. My job is to submit a PR that does what the L1 group asked of us. If the PR is not doing what is described, I'm happy to fix it. But I'm not entertaining pointless code review comments.

Regarding result = ((static_cast<unsigned>(chamber - 3) & 0x7f) / 6) + 1; // ch 3-8->1, 9-14->2, ... 1,2 -> 6
it was taken from https://github.com/cms-sw/cmssw/blob/master/DataFormats/MuonDetId/src/CSCDetId.cc#L11-L20
If you are really unhappy with the magic numbers, please fix that piece of code and then I'll copy from the updated code.

Calling the commented-out codes 'litter' is also a stretch. Most of them are single-line, and only a few of them are 3-4 lines. They don't make it 'significantly harder' to read at all.

kpedro88 · 2020-04-20T22:56:22Z

But I'm not entertaining pointless code review comments.

Please note that no developer or group is entitled to have their code included in the official repository. Your group may "take responsibility" for the code in an informal way, but once it is included in the official repository, it is everyone's responsibility - if it breaks, it must be fixed.

I deliberately included this statement in my PR review to indicate the relative importance of comments: "some of the code rule/style issues in my review seem preexisting/beyond the scope of this PR, so they can be addressed in a subsequent PR." But I am also happy to reject the PR outright if the developers feel they don't need to follow the rules at all.

jiafulow · 2020-04-20T23:09:45Z

it is everyone's responsibility - if it breaks, it must be fixed.

Except that every time the EMTF emulator broke, it was up to us to fix it.

If you feel you have the power over all the codes that we contributed, and do not want to address any concerns that your rules are bad and sometimes even pointless, then go ahead and do what you want to do. I've already provided the justifications for the select lines of codes you guys wanted to discuss. There's no reason for me to fix anything that is not broken.

kpedro88 · 2020-04-20T23:54:55Z

-1
developers are belligerent and do not understand how software management works in a large collaboration
this behavior should not be rewarded

rekovic · 2020-04-21T07:47:06Z

Coming back to this thread.

@kpedro88

@rekovic there are several reasons to avoid magic numbers in almost all circumstances. using named constants not only makes the code more readable and understandable for future maintainers by labeling numbers by their meaning, but also it enforces consistency. if a number is changed in one place, should the same number be changed somewhere else? how do i know if that 2 is the same as this other 2? this is always a future problem, not a present one, which is why the policy should always be followed whether or not a problem is "foreseen".

I agree. For the sake of consistency using constants is preferred. In some cases this could severely jeopardizes the readability, but in the long run it pays off.

rekovic · 2020-04-21T07:53:35Z

Hi @jiafulow
The requests of @kpedro88 should be addressed.

if indeed needed to keep commented out lines perhaps add description. This all can be cleaned up in the subsequent PR's part of this series when the debugging is finished.
where possible please use named constants

silviodonato · 2020-05-04T09:02:02Z

This PR has conflicts, it needs to be rebased.
CMSSW_11_1_0_pre7 will be close tomorrow.

silviodonato · 2020-05-11T15:19:16Z

moved to #29767

jiafulow added 17 commits March 20, 2020 12:33

[EMTF] Update data formats 1/3

a8ebb2a

[EMTF] Update data formats 2/3

05a3233

[EMTF] Update data formats 3/3

5a3b0fd

[EMTF] Update MuonTriggerPrimitive & GeometryTranslator 1/5

858821d

[EMTF] Update MuonTriggerPrimitive & GeometryTranslator 2/5

6f7123c

[EMTF] Update MuonTriggerPrimitive & GeometryTranslator 3/5

e3a449c

[EMTF] Update MuonTriggerPrimitive & GeometryTranslator 4/5

50d276a

[EMTF] Update MuonTriggerPrimitive & GeometryTranslator 5/5

b56bcc8

[EMTF] Update L1TMuonEndCapTrackProducer

706c7d3

[EMTF] Update EMTFSubsystemTag & EMTFSubsystemCollector 1/2

cebba8d

[EMTF] Update EMTFSubsystemTag & EMTFSubsystemCollector 2/2

f5e9d96

[EMTF] Update TrackFinder & SectorProcessor & TrackTools 1/2

52e2caa

[EMTF] Update TrackFinder & SectorProcessor & TrackTools 2/2

0f90c8e

[EMTF] Update PrimitiveSelection & PrimitiveConversion 1/4

7dbd370

[EMTF] Update PrimitiveSelection & PrimitiveConversion 2/4

1430689

[EMTF] Update PrimitiveSelection & PrimitiveConversion 3/4

4e48484

[EMTF] Update PrimitiveSelection & PrimitiveConversion 4/4

ed980ba

cmsbuild added this to the CMSSW_11_1_X milestone Mar 21, 2020

cmsbuild added code-checks-pending comparison-pending dqm-pending l1-pending orp-pending pending-signatures tests-pending labels Mar 21, 2020

jiafulow mentioned this pull request Mar 21, 2020

Add/Update interface to all the Phase-2 trigger primitives jiafulow/cmssw#1

Closed

4 tasks

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 21, 2020

cmsbuild mentioned this pull request Apr 16, 2020

From cmssw 10 2 21 #29490

Closed

kpedro88 reviewed Apr 17, 2020

View reviewed changes

cmsbuild added upgrade-rejected and removed upgrade-pending labels Apr 20, 2020

cmsbuild mentioned this pull request Apr 30, 2020

BuildFile cleanup: removed unnecessary dependencies #29608

Merged

This was referenced May 5, 2020

Clean up BuildFiles under L1Trigger/ #29685

Merged

Restructured code of againstElectronDeadECAL tauID #29627

Merged

jiafulow mentioned this pull request May 7, 2020

EMTF May 2020 emulator update #29767

Merged

cmsbuild mentioned this pull request May 8, 2020

CMS code rules for CSC digi and trigger objects #29782

Closed

silviodonato closed this May 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EMTF March 2020 emulator update #29262

EMTF March 2020 emulator update #29262

jiafulow commented Mar 21, 2020 •

edited

cmsbuild commented Mar 21, 2020

cmsbuild commented Mar 21, 2020

rekovic commented Apr 17, 2020

kpedro88 commented Apr 17, 2020

rekovic commented Apr 17, 2020

jiafulow commented Apr 17, 2020

kpedro88 Apr 17, 2020

kpedro88 Apr 17, 2020

kpedro88 Apr 17, 2020

kpedro88 Apr 17, 2020

kpedro88 Apr 17, 2020

kpedro88 Apr 17, 2020

kpedro88 Apr 17, 2020

kpedro88 Apr 17, 2020

kpedro88 Apr 17, 2020

kpedro88 Apr 17, 2020

kpedro88 commented Apr 17, 2020

rekovic commented Apr 17, 2020

kpedro88 commented Apr 17, 2020

jiafulow commented Apr 17, 2020

abrinke1 commented Apr 17, 2020

kpedro88 commented Apr 17, 2020

abrinke1 commented Apr 17, 2020

silviodonato commented Apr 20, 2020

jiafulow commented Apr 20, 2020 •

edited

kpedro88 commented Apr 20, 2020

jiafulow commented Apr 20, 2020 •

edited

kpedro88 commented Apr 20, 2020

rekovic commented Apr 21, 2020

rekovic commented Apr 21, 2020

silviodonato commented May 4, 2020

silviodonato commented May 11, 2020

		RPCDetId CreateRPCDetId() const;
		// void ImportGEMDetId (const GEMDetId& _detId);

EMTF March 2020 emulator update #29262

EMTF March 2020 emulator update #29262

Conversation

jiafulow commented Mar 21, 2020 • edited

PR description:

PR validation:

cmsbuild commented Mar 21, 2020

cmsbuild commented Mar 21, 2020

rekovic commented Apr 17, 2020

kpedro88 commented Apr 17, 2020

rekovic commented Apr 17, 2020

jiafulow commented Apr 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpedro88 commented Apr 17, 2020

rekovic commented Apr 17, 2020

kpedro88 commented Apr 17, 2020

jiafulow commented Apr 17, 2020

abrinke1 commented Apr 17, 2020

kpedro88 commented Apr 17, 2020

abrinke1 commented Apr 17, 2020

silviodonato commented Apr 20, 2020

jiafulow commented Apr 20, 2020 • edited

kpedro88 commented Apr 20, 2020

jiafulow commented Apr 20, 2020 • edited

kpedro88 commented Apr 20, 2020

rekovic commented Apr 21, 2020

rekovic commented Apr 21, 2020

silviodonato commented May 4, 2020

silviodonato commented May 11, 2020

jiafulow commented Mar 21, 2020 •

edited

jiafulow commented Apr 20, 2020 •

edited

jiafulow commented Apr 20, 2020 •

edited