-
Notifications
You must be signed in to change notification settings - Fork 448
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
DPL: add combine method to (optionally) run DataProcessors as a single #8548
Conversation
* Use concat to join multiple WorkflowSpec. * Use combine to (optionally) merge all the DataProcessors into a single one.
b4d8923
to
c89da59
Compare
@sawenzel I have seen the o2Sim G4 tests hitting timeouts in multiple places today. Did anything relevant change in the last few days? |
I will close #8529. There is no need to adapt it but it would have been nice to mention in your commit the original idea. |
Concerning o2sim: I am not aware of changes. Maybe it hangs during CCDB fetching. |
@sawenzel Apologies (and proper kudos in #8630) for the missing acknowledgement. I would have, of course, given the due credit next Monday in the WP4 meeting and in the PDP report. I would still find #8529 useful if it actually reduces the number of tasks in production. If everything is in a single executable, I think it might actually be trivial to automatically collapse parallel paths after the topological sort. |
Re CCDB, maybe it would make sense to have a log warning if CCDBApi takes too long to fetch something. |
One more thing comes to my mind. The implementation here will work nicely for specs that don't have overlapping option fields. However, I assume it will run into troubles when trying to combine processors that have common option keys. This is the case for RootTreeWriter for instance (they all have (I have solved it in my local development with keeping track of provenance of options and reinserting them on the fly during init and by auto-adjusting these option fields with the device prefix.) |
I thought about that, but what should be the behaviour? I guess we do not want to have different options for the combined case vs the non combined one, no? Shouldn't the options be adapted in the first place? |
task