Implementation of the Tprev/Tnext task. #892

leandrofloyd · 2023-09-14T08:23:00Z

Task to calculate and store the values of time difference between the Master Start hit and the previous/next hit.

Checklist:

Rebased against dev branch
My name is in the resp. CONTRIBUTORS/AUTHORS file
Followed the pull request guidlines and the Git workflow
Followed the seven rules of great commit messages

YanzhaoW

Thanks for your PR.

Besides some minor issues I mention below, from my understanding by reading the code, some values of tprev and tnext are reserved for special meanings. So if the value is equal to 5e8, it means something. If the value is equal to 5e7, it means other things. I'm not saying this is the wrong way but clearly there is a better way to do this. For example, you could use an enumerator to indicate the validity of the value:

enum class TprevTnextStatus
{
    valid,
    ms_hit,
    one_hit,
    bad_record
};

and then store this enumerator with your tprev or tnext values.

YanzhaoW · 2023-09-18T11:45:17Z

r3bbase/R3BTprevTnext.cxx

+    R3BTprevTnext::R3BTprevTnext()
+    : R3BTprevTnext("R3B Tprev/Tnext", 1)
+{
+}
+


How could this pass our clang-format checking? @klenze @jose-luis-rs

I think the problem is the lack of a semicolon in the ClassImp macro line.

For clang-format, this does not look like a finished statement.
So whatever comes next must be part of that statement and thus gets indented.

clang-format-check --fix-all did it, so...

May be put the ClassImp at the last line of the source file? This is how we usually do it.

YanzhaoW · 2023-09-18T11:46:10Z

r3bbase/R3BTprevTnext.cxx

+    : FairTask(name, iVerbose)
+    , fMSOffsetPar(nullptr)
+    , fTprev(0.0)
+    , fTnext(0.0)
+    , fSamplerMapped(nullptr)
+    , fSamplerMSMapped(nullptr)
+    , fR3BEventHeader(nullptr)
+    , fTpat(1024.0)
+    , delta_clk(1.0)
+{
+}


As suggested in the last PR, please use the inline-initialization format.

While I agree with @YanzhaoW in principle, I would also like to point out that this is among the bad practices used in the ./template/ directory. I think it would be worthwhile to clean that up, then we are in a much better position to display rightful indignation if PRs still use that :-)

Is the inline initialization here only changing the comma position to the end of the lines instead of the beginning?

You could see the correct way here (second one is preferred). It's very simple. You just need to define the default values inside the class definition. ;D

YanzhaoW · 2023-09-18T11:47:17Z

r3bbase/R3BTprevTnext.cxx

+{
+}
+
+R3BTprevTnext::~R3BTprevTnext() { LOG(info) << "R3BTprevTnext: Delete instance"; }


Do we really need a log for the destructor?

No. We also do not need a destructor. But again, this should be fixed in ./template/

Yes, the code needs a lot of updates

I can remove this destructor

Also don't forget to remove copy & move constructor and assignmentl. See rule of 5.

YanzhaoW · 2023-09-18T12:07:53Z

r3bbase/R3BTprevTnext.cxx

+        fR3BEventHeader->SetTprev(fTprev);
+        fR3BEventHeader->SetTnext(fTnext);


This is a bit unconventional. We usually don't change the data read from the TTree.

I think this is probably the least painful way to go about it.

IMHO it makes much more sense to store these values in the event header than to create yet another TCA containing just a single entry of yet another data class, which everyone would have to fetch from FRM separately.

The other alternative would be to squeeze this code into whatever fills the rest of the event header (probably in r3bsource), which would also be messy.

@klenze made his point here

YanzhaoW · 2023-09-18T12:27:19Z

r3bbase/R3BTprevTnext.cxx

+            {
+                fTprev = tpn * 10.0; // in ns


What is the reason behind this magic number 10.0?

Yes. It's correct

The trloii sampler counts clock cycles of a 100MHz clock.
That means that one clock cycle is 10ns.
(We have discussed using a uniform unit system before, but I don't think this is the correct place to start with that.)

Again @klenze explained it

In this case, maybe define a named constant on the top:

constexpr auto CLOCK_PERIOD = 10.; // ns

And then use this variable in the code:

fTprev = tpn * CLOCK_PERIOD;

YanzhaoW · 2023-09-18T12:31:02Z

r3bbase/R3BTprevTnext.cxx

+        fTprev = 1e9; // Initialization with a different value after the first tests
+        fTnext = 1e9; // If you have this value in your tprev/tnext is because the previous or next hit is inside the
+                      // delta_clk range but is not a MS
+        for (Int_t i = 0; i < sampHits; ++i)


Maybe put this for-loop into another member function and give it a nice name indicating the meaning?

klenze

The question which values we should use to indicate invalid time values deserves some consideration.

I think people will mostly use Tprev and Tnext in cuts where they select events which are isolated:
if (Tprev>1000 && Tnext > 1000) { // Fill your histograms or whatever }
The default values you use for ((sampHits < 1) || (sampmsHits != 1)) are nice invalid values which will clearly indicate an unsafe event.

However, all of your other failure values are big positive numbers, claiming that there is no event for at least half a second around the one in question. I think this is overly optimistic.

For an initial value of fTPrev, you could remember the value fLastWrts=GetWRTS()-10*uint64_t(tpn) (with your sign convention) of the last Exec call in a member variable. Then your fTPrev simply is GetWrts()-fLastWrts(). (If fLastWrts has never been set, just invalidate fTPrev by setting it negative.)

Your fTnext is more troublesome, because with a single pass over the tree you can't know what comes next. A defensible value would be (offset-1)*10.0. After all, it took at least that long between the trigger which caused the readout to hit the sampler and the sampler actually bank switching (or whatever samplers do) for the readout.

Are you aware that the sampler clock wraps around every 43 seconds, and are ok with your difference values being widely off whenever that happens?

klenze · 2023-09-18T15:57:03Z

r3bbase/R3BTprevTnext.cxx

+    R3BTprevTnext::R3BTprevTnext()
+    : R3BTprevTnext("R3B Tprev/Tnext", 1)
+{
+}
+


I think the problem is the lack of a semicolon in the ClassImp macro line.

For clang-format, this does not look like a finished statement.
So whatever comes next must be part of that statement and thus gets indented.

klenze · 2023-09-18T16:00:29Z

r3bbase/R3BTprevTnext.cxx

+    : FairTask(name, iVerbose)
+    , fMSOffsetPar(nullptr)
+    , fTprev(0.0)
+    , fTnext(0.0)
+    , fSamplerMapped(nullptr)
+    , fSamplerMSMapped(nullptr)
+    , fR3BEventHeader(nullptr)
+    , fTpat(1024.0)
+    , delta_clk(1.0)
+{
+}


While I agree with @YanzhaoW in principle, I would also like to point out that this is among the bad practices used in the ./template/ directory. I think it would be worthwhile to clean that up, then we are in a much better position to display rightful indignation if PRs still use that :-)

klenze · 2023-09-18T16:00:57Z

r3bbase/R3BTprevTnext.cxx

+{
+}
+
+R3BTprevTnext::~R3BTprevTnext() { LOG(info) << "R3BTprevTnext: Delete instance"; }


No. We also do not need a destructor. But again, this should be fixed in ./template/

klenze · 2023-09-18T16:02:30Z

r3bbase/R3BTprevTnext.cxx

+{
+
+    fTprev = -1.0e3; // Initializaton of Tprev and Tnext
+    fTnext = -1.0e3; // If in the end you have this value is because this is an offspill or SAMP and/or SAMPMS are not


These should be local variables.

You are right...
...again

klenze · 2023-09-18T16:09:34Z

r3bbase/R3BTprevTnext.cxx

+        fR3BEventHeader->SetTprev(fTprev);
+        fR3BEventHeader->SetTnext(fTnext);


I think this is probably the least painful way to go about it.

IMHO it makes much more sense to store these values in the event header than to create yet another TCA containing just a single entry of yet another data class, which everyone would have to fetch from FRM separately.

The other alternative would be to squeeze this code into whatever fills the rest of the event header (probably in r3bsource), which would also be messy.

klenze · 2023-09-18T16:15:27Z

r3bbase/R3BTprevTnext.cxx

+            {
+                fTprev = tpn * 10.0; // in ns


The trloii sampler counts clock cycles of a 100MHz clock.
That means that one clock cycle is 10ns.
(We have discussed using a uniform unit system before, but I don't think this is the correct place to start with that.)

klenze · 2023-09-18T16:34:49Z

r3bbase/R3BTprevTnext.cxx

+                    fTnext = 5e8; // If your Tnext==5e8, your last hit is MS
+                    break;
+                }
+            }


While I am not sure that the above code is correct as is, I am sure that the code could be simplified.
First, initialize both fTnext and fTprev to invalid. bool sawMS=false;
Then, loop.
Calculate auto tpn = SAMPTime - SAMPMSTime - MSOffset; (so it increases).
As long as tpn is smaller than -1, upate fTprev to -10*tpn.
If abs(tpn)<=1, set a bool sawMS to one.
If tpn>1, set fTnext and break.

This way, iff fTPrev or fTNext or both are never set, they automatically contain the invalid value.

Also, if sawMS is false for more than a small fraction of events, then something terrible has happened (likely the offset is wrong). We should complain periodically with LOG(error).

Ok, seems like a good idea, but again, I need some time to understand completely. But I will come with something

leandrofloyd · 2023-09-19T07:20:57Z

r3bbase/R3BTprevTnext.cxx

+        }
+    }
+    else
+    {


You are right

klenze · 2023-09-18T16:44:36Z

r3bbase/R3BTprevTnext.cxx

+
+    if (fR3BEventHeader->GetTpat() < fTpat && fR3BEventHeader->GetTrigger() == 1 && fR3BEventHeader->GetTpat() > 0)
+    {
+        if (sampHits == 1)


No need to handle that case specially. Just let the for loop roll over one entry.

This one I need to think more about to fully understand

leandrofloyd · 2023-09-19T08:17:36Z

Thanks for your PR.

Besides some minor issues I mention below, from my understanding by reading the code, some values of tprev and tnext are reserved for special meanings. So if the value is equal to 5e8, it means something. If the value is equal to 5e7, it means other things. I'm not saying this is the wrong way but clearly there is a better way to do this. For example, you could use an enumerator to indicate the validity of the value:
enum class TprevTnextStatus
{
    valid,
    ms_hit,
    one_hit,
    bad_record
};
and then store this enumerator with your tprev or tnext values.

I will try to do that, good idea, thanks.

leandrofloyd · 2023-09-19T08:28:29Z

The question which values we should use to indicate invalid time values deserves some consideration.

Fully agree, this is one of the things I really want to be discussed here before the code is accepted.

I think people will mostly use Tprev and Tnext in cuts where they select events which are isolated: if (Tprev>1000 && Tnext > 1000) { // Fill your histograms or whatever } The default values you use for ((sampHits < 1) || (sampmsHits != 1)) are nice invalid values which will clearly indicate an unsafe event.

However, all of your other failure values are big positive numbers, claiming that there is no event for at least half a second around the one in question. I think this is overly optimistic.

You are right here, I just used the numbers I used based on the test cases I used, but we can discuss what is better for general purposes.

For an initial value of fTPrev, you could remember the value fLastWrts=GetWRTS()-10*uint64_t(tpn) (with your sign convention) of the last Exec call in a member variable. Then your fTPrev simply is GetWrts()-fLastWrts(). (If fLastWrts has never been set, just invalidate fTPrev by setting it negative.)

I really like your idea, but I will need more time to fully understand, maybe we can discuss that at GSI, the problem is, I am out on vacation for 3 weeks starting tomorrow (I was not expecting to have 5, 6 weeks of wait for the "Offset part" of the code).

Your fTnext is more troublesome, because with a single pass over the tree you can't know what comes next. A defensible value would be (offset-1)*10.0. After all, it took at least that long between the trigger which caused the readout to hit the sampler and the sampler actually bank switching (or whatever samplers do) for the readout.

Need more time here again to process the information.

Are you aware that the sampler clock wraps around every 43 seconds, and are ok with your difference values being widely off whenever that happens?

Yes, initially I made some checks and corrections related to that, but, again, maybe this is a good thing to discuss with you in person, but, again (again), I am going on vacation tomorrow, so we can discuss that after I return.

Thank you very much for this discussion, this is the kind of thing I was expecting to discuss here.

jose-luis-rs · 2023-10-26T13:04:03Z

Hello @leandrofloyd
Do you plan to do more modifications?
Cheers

leandrofloyd · 2023-10-26T13:26:29Z

Hello @jose-luis-rs
Yes, I will have some modifications yet.
Cheers

leandrofloyd · 2023-11-02T10:17:54Z

@jose-luis-rs @klenze @YanzhaoW Please check this new code. There are some more things I want to discuss, like the values for the cases, the use of WR, etc... But please take a look at how the code looks now and we can start discussions.

r3bbase/R3BTprevTnext.h

r3bbase/R3BTprevTnext.cxx

YanzhaoW · 2023-11-02T14:31:52Z

r3bbase/R3BTprevTnext.cxx

+    if (fR3BEventHeader->GetTpat() < fTpat && fR3BEventHeader->GetTrigger() == 1 && fR3BEventHeader->GetTpat() > 0)
+    {
+        if (sampmsHits == 1 && sampHits > 0)
+        {
+            SAMPMSMapped = dynamic_cast<R3BSamplerMappedData*>(fSamplerMSMapped->At(0));
+            SAMPMSTime = SAMPMSMapped->GetTime();
+            for (Int_t i = 0; i < sampHits; ++i)
+            {
+                SAMPMapped = dynamic_cast<R3BSamplerMappedData*>(fSamplerMapped->At(i));
+                SAMPTime = SAMPMapped->GetTime();
+                auto tpn = SAMPTime - SAMPMSTime - MSOffset;
+                if (tpn < -delta_clk)
+                {
+                    fTprev = -clock_period * tpn;
+                }
+                if (abs(tpn) <= 1)
+                {
+                    sawMS = true;
+                    ++dMScounter;
+                }
+                if (tpn > delta_clk)
+                {
+                    fTnext = clock_period * tpn;
+                    break;
+                }
+            }
+        }
+    }
+    if (sawMS)
+    {
+        if (fTprev == -10.0)
+        {
+            fTprev = 43.1e9; // MS is the first hit
+        }
+        if (fTnext == -10.0)
+        {
+            fTnext = 43.1e9; // MS is the last hit
+        }
+        if (dMScounter > 1)
+        {
+            if (fTprev == 43.1e9)
+            {
+                fTprev = 0; // Tprev inside the delta_clk
+            }
+            if (fTnext == 43.1e9)
+            {
+                fTnext = 0; // Tnext inside the delta_clk
+            }
+        }
+        if (dMScounter == 1 && fTprev == 43.1e9 && fTnext == 43.1e9)
+        {
+            fTprev = -20.0; // Only one hit in the event
+            fTnext = -20.0;
+        }
+    }


(ignore this if you don't have time to fix)

Personally when I read this section of code, I feel very difficult to understand what's going on. This could be caused by lots of if statements and magic floating point values. Normally a function contains at most 2 to 3 if statements and anything more than that makes the code hard to read and understand. Unfortunately, I can't tell you exactly how to solve this issue as it requires lots of advanced experiences such as code design and architeture. I'm curious how @klenze feels about this.

I guess this is ready, other improvements can be done later in a new PR

klenze

General questions:
Why do you refuse to handle certain tPats?
The way I see it, either we have a main DAQ trigger, in which case Tprev/Tnext can always be set as usual, or we don't, in which case all the scalers will be empty and we will default to setting Tprev/Tnext to something invalid.

Also, did you double-check that our event header resets the Tprev/Tnext to something invalid (zero?) for each new event?

Sorry for taking so long to get back to you.

klenze · 2023-11-02T12:21:28Z

r3bbase/R3BTprevTnext.cxx

+    fSamplerMapped = dynamic_cast<TClonesArray*>(rootManager->GetObject("SamplerMapped"));
+    if (fSamplerMapped == nullptr)
+    {
+        return kFATAL;


I would prefer if you did one of the following:

assert(fSamplerMapped)

if () { LOG(fatal)<< "SamplerMapped not found or wrong type.";}

if () {LOG(error) << "..."; return kFATAL; }

If you just return kFATAL, FairRoot is (hopefully) going to report which initialization of a Task failed, but not why exactly.

klenze · 2023-11-02T12:33:44Z

r3bbase/R3BTprevTnext.cxx

+{
+    Double_t fTprev = -10.0; // Initialization with invalid values.
+    Double_t fTnext = -10.0;
+    Int_t dMScounter = 0;


First, please add // number of sampler hits in coincidence with MS
Second, if we use this, sawMS should be removed and replaced by dMScounter>0.
Third, do we actually need to handle the special case where LOS saw two ions within 20ns? My feeling is no. In that case, running the algorithm as usual should result in Tprev or Tnext being really small, which will in turn make the users reject the hit as an obvious pileup.

klenze · 2023-11-02T12:40:42Z

r3bbase/R3BTprevTnext.cxx

+    Double_t SAMPTime = 0;
+    Double_t SAMPMSTime = 0;
+    R3BSamplerMappedData* SAMPMapped = nullptr;
+    R3BSamplerMappedData* SAMPMSMapped = nullptr;


Please declare SAMPTime to SAMPMSMapped in the loop when you assign them.
(@YanzhaoW would probably want you to declare them auto and auto*)

klenze · 2023-11-02T12:43:27Z

r3bbase/R3BTprevTnext.cxx

+                {
+                    fTprev = -clock_period * tpn;
+                }
+                if (abs(tpn) <= 1)


if (abs(tpn) <= delta_clk)

klenze · 2023-11-04T13:40:35Z

r3bbase/R3BTprevTnext.cxx

+}
+
+void R3BTprevTnext::Exec(Option_t* /*opt*/)
+{


constexpr auto NOTFOUND = -10.0;
constexpr double WRAP_PERIOD = (1L<<32)*CLOCK_PERIOD; //42.9 seconds
constexpr auto ONLYHIT = -20.0;

If you ever return these, I would make the constants a public part of the class interface. If you only use them internally, put them with CLOCK_PERIOD.

klenze · 2023-11-04T13:53:52Z

r3bbase/R3BTprevTnext.cxx

+            }
+        }
+    }
+    if (sawMS)


I am still not happy with the interpretation of sawMS, fTprev and fTnext here.

If sawMS is false, we should probably set Tnext and Tprev to some negative header constant INVALID_NOMS, it it keeps happening, we should tell the user that their offset is likely wrong.

Otherwise, if fTprev is invalid, we could set it to an estimate on how long our DAQ dead time is minimally. (The fancier way to track samplers over multiple readouts can be implemented later.) Perhaps make it a header constant on the order of 2000ns?

If fTnext is invalid, the defensible value is the offset, which is the delay through the trigger matrix. (If another LOS trigger had appeared before the MS arrived at the sampler, it would have been recorded, after all.)

43.1e9 aka "no triggers for 43 seconds" should not be used as an output ever.

leandrofloyd · 2023-11-14T09:54:29Z

This version was made after a meeting with @klenze where he explained some of his comments and presented me with some ideas.
I tried to implement every suggestion (if the code change doesn't change completely the part of the suggestion, of course).

klenze

One thing to consider might be to make the value for "no (prev/next) hit found" part of the calibration process (possibly as two separate values).

If I remember correctly, there is a clear edge in the tnext histogram. For Tprev, things are less clear cut (but also less critical, because we were recording for a substantial fraction of the dead time, I hope. Or we just use the info from the last readout together with the WRTS, eventually.)

But this does not feel critical and can be done in a later PR. Please consider changing my small nitpicks in the code, write an explanation when the NoRecord value will be used and what it should be set to as a comment to the Setter method, and this commit is a-ok for merging as far as I am concerned.

klenze · 2023-11-18T20:25:44Z

r3bbase/R3BTprevTnext.h

+    /** Virtual method Exec **/
+    void Exec(Option_t* opt) override;
+
+    void SetOne_hit(Double_t One_hit_position) { fOne_hit = One_hit_position; }


SetNoRecordTimeGap?

klenze · 2023-11-18T20:31:08Z

r3bbase/R3BTprevTnext.h

+    TClonesArray* fSamplerMSMapped = nullptr;
+    R3BEventHeader* fR3BEventHeader = nullptr; /**< Event header - input data. */
+    Double_t fDelta_clk = 1.0;
+    Double_t fOne_hit = 3e6;


=2e3; // 2us

If the user does not provide a safe value, assume a smallish value which will make them reject more events rather than a gargantuan value which will see them include these hits in every cut.

klenze · 2023-11-18T20:34:32Z

r3bbase/R3BTprevTnext.cxx

+constexpr auto INVALID_TPTN = -10;   // Assigned value for invalid or nonexistent TPrev or TNext
+constexpr auto ERROR_MULTI_MS = -20; // Assigned value for events where there are multiple MS
+constexpr auto INVALID_EVENT = -40;  // Assigned value for events where the MS was not correctly written
+constexpr auto FIRST_LAST_HIT = 0;   // Assigned value for events where the first/last hit is the MS


any of these constants which are visible to the user should be placed as public static constexpr members of the class in the header.

klenze · 2023-11-18T20:44:43Z

r3bbase/R3BTprevTnext.cxx

+        if (fTprev > 0 && fTnext < 0)
+        {
+            fTnext = FIRST_LAST_HIT;
+        }


I would replace the last if block with:

else if (dMScounter == 1) { if (fTprev == INVALID_TPTN) fTprev = fOne_hit; if (fTnext == INVALID_TPTN) fTnext = fOne_hit; } fR3BEventHeader->SetTprev(fTprev); fR3BEventHeader->SetTnext(fTnext);

That way, whenever there is no previous/next hit, the "NoRecord" value will be used. For the special case of one hit (no previous and no next hit), both of these will automatically be set to that value.

leandrofloyd · 2023-11-20T13:30:59Z

@klenze and @YanzhaoW I hope that this version fulfils everything that both of you have asked for.
@klenze did I understand your comments correctly?

klenze · 2023-11-21T12:00:13Z

Great, thanks!

@jose-luis-rs: This is ready for merging as far as I am concerned.

jose-luis-rs · 2023-11-21T12:05:48Z

Thanks!

leandrofloyd force-pushed the tptn branch from c37cbd6 to 258146e Compare September 14, 2023 08:30

YanzhaoW reviewed Sep 18, 2023

View reviewed changes

klenze suggested changes Sep 18, 2023

View reviewed changes

leandrofloyd force-pushed the tptn branch 2 times, most recently from ec2433c to ae0231f Compare November 2, 2023 10:04

YanzhaoW reviewed Nov 2, 2023

View reviewed changes

leandrofloyd force-pushed the tptn branch from ae0231f to 488b300 Compare November 3, 2023 10:00

klenze suggested changes Nov 4, 2023

View reviewed changes

klenze mentioned this pull request Nov 11, 2023

About clang-tidy and code standard #907

Closed

leandrofloyd force-pushed the tptn branch from 488b300 to 1c2b991 Compare November 14, 2023 09:35

leandrofloyd force-pushed the tptn branch from 1c2b991 to ecd971c Compare November 15, 2023 10:54

klenze suggested changes Nov 18, 2023

View reviewed changes

Implementation of the Tprev/Tnext task.

f2bc50e

leandrofloyd force-pushed the tptn branch from ecd971c to f2bc50e Compare November 20, 2023 13:17

jose-luis-rs merged commit 71a9f2e into R3BRootGroup:dev Nov 21, 2023
5 checks passed

YanzhaoW mentioned this pull request Nov 28, 2023

[FairRoot 19] Apply Modern CMake and clear dependencies #911

Merged

4 tasks

		fR3BEventHeader->SetTprev(fTprev);
		fR3BEventHeader->SetTnext(fTnext);

Implementation of the Tprev/Tnext task. #892

Implementation of the Tprev/Tnext task. #892

Conversation

leandrofloyd commented Sep 14, 2023 • edited Loading

YanzhaoW left a comment • edited Loading

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klenze left a 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

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leandrofloyd commented Sep 19, 2023

leandrofloyd commented Sep 19, 2023 • edited Loading

jose-luis-rs commented Oct 26, 2023

leandrofloyd commented Oct 26, 2023

leandrofloyd commented Nov 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klenze left a 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

leandrofloyd commented Nov 14, 2023

klenze left a 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

leandrofloyd commented Nov 20, 2023

klenze commented Nov 21, 2023

jose-luis-rs commented Nov 21, 2023

leandrofloyd commented Sep 14, 2023 •

edited

Loading

YanzhaoW left a comment •

edited

Loading

leandrofloyd commented Sep 19, 2023 •

edited

Loading