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

Glm/mlt #293

Merged
merged 4 commits into from
Apr 5, 2024
Merged

Glm/mlt #293

merged 4 commits into from
Apr 5, 2024

Conversation

glehmannmiotto
Copy link
Contributor

MLT restructuring (goes together with appdal schema changes)

Copy link
Contributor

@ArturSztuc ArturSztuc left a comment

Choose a reason for hiding this comment

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

LGTM!

<< pending_td.readout_start << "/" << pending_td.readout_end;
//m_td_paused_tc_count += pending_td.contributing_tcs.size();
TLOG_DEBUG(1) << "Triggers are paused. Not sending a TriggerDecision for TD with timestamp and type "
<< ts << "/" << tt;
Copy link
Contributor

Choose a reason for hiding this comment

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

The type is a bitword (there can be multiple TCs of different types in TD), and I'm not sure if auto appreciates it, or just converts e.g. 000100010 (say, prescale & Timing types in this TD) to e.g. 34 (meaningless to reader).

Doesn't need to be resolved in this PR though, though maybe a TODO comment could be useful.

TLOG_DEBUG(1) << "The DFO is busy. Not sending a TriggerDecision for candidate timestamp "
<< pending_td.contributing_tcs[m_earliest_tc_index].time_candidate;
TLOG_DEBUG(1) << "The DFO is busy. Not sending a TriggerDecision with timestamp and type "
<< ts << "/" << tt;
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

return ignore;
}

std::bitset<16>
Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking out loud, all these 16s will need to be changed to 64s when merging v4 into develop.

I think v4 and v5 diverged so much, that maybe there's some motivation to NOT do a standard v4 merge into develop, and instead just re-implement whatever v4 features we had into v5 from scratch. It might be easier...

@glehmannmiotto glehmannmiotto merged commit 58111a5 into develop Apr 5, 2024
1 of 2 checks passed
@glehmannmiotto glehmannmiotto deleted the glm/mlt branch April 5, 2024 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants