[Documentation Code Camp] - Added introductory section for queue documentation - Part 1 #889
Conversation
marcus-oscarsson
force-pushed
the
mo-march-queue-doc
branch
from
9f3689a to
58254e6
Compare
|
This is still very much in progress, Ill try to add a bit more introduction and some figures that might make things easier to understand. Then Ill go on with some more details that will point out where things are implemented and how new tasks are created. |
|
If you think about something that you'd like to know Id try to add it, I'm not promising that it will be any clearer but it least it will be documented ;) |
|
I do have one. As I understood the old system, there was a single source of truth, specifically the tree of QueueModelObjects. But (again as I understood it) MxcubeWeb introduced a new data structure that served as a competing source of truth that could be queried or modified independently. I'd like to understand what both those structures are and how they interact. |
|
Ill try to clarify that, the queue is the same in both Qt and Web, still the same old tree of |
There was a problem hiding this comment.
Same suggestion I made for the other pull requests: when dealing with markup I tend to prefer Semantic Line Breaks. It is a personal preference, as far as I know we do not have a policy for this, so feel free to follow it, to whatever degree you feel comfortable with, or not at all.
marcus-oscarsson
force-pushed
the
mo-march-queue-doc
branch
4 times, most recently
from
3f6c69b to
2f2ee99
Compare
|
I'm sorry @axelboc but I'm not done yet its still very much work in progress this but ill make a diagram as well |
docs/source/dev/queue.md
Outdated
| ### Differences between the Web and the Qt user interfaces | ||
| When the Web interface was designed, the scientists wanted to change some aspects of how the queue operates and how it is presented to the user. The decision was to work on one sample at a time, rather than for the entire queue, as is done in the Qt version. This also means that the Web interface is centered around displaying the contents of the queue for one sample. It was also decided to not display `DataCollectionGroupQueueEntry` in the queue. The `DataCollectionGroupQueueEntry` still exists in the Web interface, but there is no view created for it. | ||
|
|
||
| On a technical level, there is a tight coupling between the `QueueEntry` base class to a Qt view object that makes the creation of the object slightly different between the two user interfaces (Qt and Web). To handle the tight coupling to Qt a `Mock` object used in the Web version instead of a Qt view and the handling of the updating of the view in the web version handled through signals passed over websockets to the client. |
There was a problem hiding this comment.
To handle the tight coupling to Qt a Mock object used in the Web version instead of a Qt view and the handling of the updating of the view in the web version handled through signals passed over websockets to the client.
This sentence might be better if split in two????? So the Web has a Mock object to mimic the Qt?
There was a problem hiding this comment.
Yes, you are right. That sentence is a a bit convoluted
There was a problem hiding this comment.
"This sentence might be better if split in two????? So the Web has a Mock object to mimic the Qt?"
Yes
marcus-oscarsson
force-pushed
the
mo-march-queue-doc
branch
6 times, most recently
from
fe40da5 to
f194459
Compare
|
AH, one question here: In the diagram it is shown with Samples always at the top ()makes sense) and with only one level of DataCollectionGroup. Is that a rule, just common practice, or a coincidence? When running multisweep workflow experiments, it arguably makes more sense to have multiple levels or groupings, a group for the entire workflow, containing another group for teh characterisation, and yet another group for the actual acquisition sweeps. And indeed this is how I have coded the GPhL workflow. I think we need to decide whether we accept multiple levels of groupings or not, and make it explicit in the documentation, otherwise people will start making incorrect assumptions and different bits of code can clash. |
|
|
||
| This design was thought to map well to the semantics of how data is collected with a sample changer and the upload of metadata to LIMS. The sample changer has a number of samples, each having a set of data collections (child nodes). The results are uploaded to the lims system after each data collection, (`post_execute`). Workflow engines can further group native collection protocols into grouping nodes, which in turn can be used as building blocks for even more complex collection strategies. | ||
|
|
||
| ```{figure} /assets/queue_execute_methods.svg |
There was a problem hiding this comment.
Thanks, I'm aware, I'm still working on it. Also trying to get the scaling to work
|
@rhfogh, I'm still working on this section and ill try to make some notes on whats common practice between what is possible. There is no decision on how deep one can nest data collection groups or samples for that matter. Its left open by purpose. The conventional one crystal type per pin with sample changer collection is whats shown in the figure, Ill make a note on that. One can imagine that for instance a plate sample changer where one have two levels of samples plate and crystal, I'm not saying that thats how it should be done but its a possibility. For workflows nested groups could make sense and even for multi sweep collections. |
|
OK, If you will put in something explicit about allowing nested groups, I'll remove my 'Request Changes' immediately and leave the rest to you. I made the point because (until I modified it) there was code assuming that the Sample was always two levels up from the DataCollection. I still need to check with Olof exactly what the rules and assumptions about grouping are for MASSIF-1 operation. |
|
I'm not sure how to approach this because I make some changes and push mostly to see how it renders, but then I get comments on certain things that are still in progress. As its draft Ill try to answer your general questions regarding the queue like the one @rhfogh asked, and the rest is still very much subject to change. |
|
You could mark it as WIP, if you prefer that we wait with the comments. |
marcus-oscarsson
force-pushed
the
mo-march-queue-doc
branch
from
f194459 to
a7cad59
Compare
|
We just got mixed up on what "Draft" means. Marcus opened this to show progress and to preview his work, not to request a review. As discussed during the meeting this morning, "Draft" means that it's not yet ready for review (unless the opener specifically asks for feedback). The removal of "Draft" status by the opener means that reviews are welcome. |
|
I happy for the broader questions like the one @rhfogh asked or general questions about the queue that you'd like to know about, it helps me to narrow down on some details. Just to avoid that we waste our time on things that will most likely change. I also mark it as WIP ;) |
marcus-oscarsson
changed the title
marcus-oscarsson
force-pushed
the
mo-march-queue-doc
branch
3 times, most recently
from
af8affc to
b52f521
Compare
marcus-oscarsson
force-pushed
the
mo-march-queue-doc
branch
2 times, most recently
from
674a9df to
fb2bba8
Compare
marcus-oscarsson
changed the title
marcus-oscarsson
changed the title
|
I believe this is starting to get usable, its not complete yet I will add a few sections in a near future. I plan on adding more on how the execution works and then Ill add something about the new queue design and the drawbacks of the current design. However, thats for Part 2 :) |
|
Nice with diagrams. |
docs/source/dev/queue.md
Outdated
| ``` | ||
|
|
||
| **_NOTE:_** There are no constraints on the structure of the tree. However, today: the top level always contains sample nodes, | ||
| and the nesting occurs at the group level. The depth of nesting is currently maximum 3 - Sample - |
There was a problem hiding this comment.
That is not actually the case for GPhL workflow - we nest more deeply. Maybe we should change that. This probably needs discussion, to make sure that everybody either is or is not ready for deeper nesting.
There was a problem hiding this comment.
That's not an issue, If you know how many levels you nest Ill update with that figure or phrase it like "the most common nesting depth is 3" or similar
There was a problem hiding this comment.
So, I added a "disclaimer" for workflows, but let me know if you rather that we add a figure.
docs/source/dev/queue.md
Outdated
| group_entry.enqueue(dc_entry) | ||
| ``` | ||
|
|
||
| Each type of data collection protocol has a piece of code similar to the code above associated with it. In the Web version, this is done in mxcubeweb.components.queue.py in the methods called add_**task_type**, for instance `add_data_collection`. In the Qt version, the creation is done in two different places: first in the widget where the queue model object is created, and then through `QueueModel.view_created`. |
There was a problem hiding this comment.
I think we should say explicitly that in the Qt version enqueueing the queue entry is done automatically when the qmo is added to the queue, via the QueueModel.add_child function, which sends out a child_added signal. It seems - to a very quick look - that the same is the case in the web version, via the function components.queue.Queue.queue_model_child_added. Maybe this should have a more prominent place in thedocumentation. Actually, is it the rule or an exception when enqueue is called explicitly rather than via signal?
There was a problem hiding this comment.
Its not entirely correct for the web version because its only the case for collections added through workflows that use the signal, the rest is done through enqueue. Ill see what I can do
There was a problem hiding this comment.
Thanks, I tried to make clarify a bit, let me know what you think ?
marcus-oscarsson
force-pushed
the
mo-march-queue-doc
branch
2 times, most recently
from
6e164c6 to
9f9a17c
Compare
marcus-oscarsson
force-pushed
the
mo-march-queue-doc
branch
from
9f9a17c to
b279f56
Compare
No description provided.