sanger / sequencescape

Web based LIMS
MIT License
80 stars 32 forks source link

Y24-040 Specify 2 lanes only for NovaSeq X Plus 1.5B flowcell #4068

Open neilsycamore opened 3 months ago

neilsycamore commented 3 months ago

User story As a High Throughput Short Read Sequencing team lead (Tris), I want the system to check the NovaSeqX submitted samples for 'Flow cell Type' and 'Permitted number of lanes/read lengths' before a sequencing batch is created SO THAT the additional monetary expense incurred due to incorrect sample selection can be avoided.

For the NovaSeqX there following flow cell options:

  1. 1.5B (Two lane), - Primary requirement for this story
  2. 10B (Eight lane),
  3. 25B (Eight lane)

Who are the primary contacts for this story Tris

Who is the nominated tester for UAT Tris

Acceptance criteria To be considered successful the solution must allow:

  1. If I select requests where the flow cell requested is 1.5B, I am only permitted to create a batch for exactly 2 requests i.e. 2 lanes (no more no less).
  2. If I select requests where the flow cell requested is something else, behaviour is unchanged i.e. the system allows for 8 lane requests for both 10B and 25B flow cell types. NB: There was a similar requirement for NovaSeq 6000 added to the story https://github.com/sanger/sequencescape/issues/3770. It would be worth confirming if the change was implemented or still needs to be implemented.
  3. If I select requests where the requested read lengths are different, I am only permitted to create a batch for the same read lengths. (This is similar to that in NovaSeq 6000 pipeline)
  4. If I select requests in a different sequencing pipeline inbox, the behaviour is unchanged.
  5. I cannot create a batch containing requests of different types of flow cells types(10B, 25B, 1.5B) (NB: Similar to https://github.com/sanger/sequencescape/issues/3819. Need to confirm if can be reused)
  6. I can see an updated message prompted on the message banner informing about the (Can reuse similar wording as in NovaSeq 6000 pipeline)
    • permitted lanes
    • limitations around mixing flow cell types based on users' selection
    • limitations around mixing read lengths based on users' selection

image

Additional context

Comments from Tris and RT raised RT799336

  1. When selecting requests from the NovaSeq X pipeline inbox the number of lanes permitted for a 1.5B flow cell should be 2 (no more no less).
  2. Currently, SS is restricting batch creation. Also, the message displayed is not relevant to the scenario supporting 1.5B flow cells.
  3. Restrict mixing of requests (sample pool) for different types of flow cells (10B, 25B, 1.5B) on the same batch. The intention is to build capability for error handling (Good to Have)

Blocking dependency #4135

KatyTaylor commented 2 months ago

Some investigation:

There's a validation method in batch.rb as follows:

  def batch_meets_minimum_size
    if min_size && (requests.size < min_size)
      errors.add :base, "You must create batches of at least #{min_size} requests in the pipeline #{pipeline.name}"
    end
  end

References these fields: Screenshot 2024-04-16 at 15 22 31

Looks like for this story, validation being at a pipeline level is not sufficient, it needs to be specific to the combination of pipeline and flowcell type?

KatyTaylor commented 2 months ago

Previous relevant PR (changes for NovaseqX) - https://github.com/sanger/sequencescape/pull/3794 and where flowcell_types was added - https://github.com/sanger/sequencescape/pull/3649

KatyTaylor commented 2 months ago

The 'Flowcell type' referenced by the story is recorded in the request_metadata table as requested_flowcell_type. It is displayed in the 'Flowcell requested' column in the sequencing inbox table (https://training.sequencescape.psd.sanger.ac.uk/pipelines/52), e.g.:

Screenshot 2024-04-17 at 14 20 54

KatyTaylor commented 2 months ago

I think the requirement "Restrict mixing of requests (sample pool) for different types of flowcells (10B, 25B, 1.5B) on the same batch." is already met by the below code, although I haven't tested it.

  def requests_have_same_flowcell_type
    unless pipeline.is_flowcell_type_consistent_for_batch?(self)
      errors.add :base, "The selected requests must have the same values in their 'Flowcell Requested' field."
    end
  end
KatyTaylor commented 2 months ago

@SujitDey2022 could we have some acceptance criteria? Would it be something like:

When selecting requests from the NovaSeq X pipeline inbox (Pipelines tab > NovaSeqX PE):

KatyTaylor commented 2 months ago

I've sized this as small as I think it's just adding some more server-side validation in the batch.rb class.

SujitDey2022 commented 2 months ago

Thanks @KatyTaylor I was intending to refine the story and acceptance criteria Today. Will get it done...

neilsycamore commented 2 months ago

@KatyTaylor @SujitDey2022 Conversation 15/03/2024 tw6 & tkb • Will you always use 2 flow lanes or is there a possibility that only 1 flow lane could be used? For the NovaSeq X Plus there will only be the following options: 1.5B (Two lane), 10B (Eight lane), 25B (Eight lane) • When do you foresee the next 1.5B flowcell run happening? Our top priority is to get this validation pushed through somehow but after that I have no further visibility but as this is now a product which Illumina provide II expect Faculty will be wanting this option shortly.

SujitDey2022 commented 2 months ago

Thanks @neilsycamore, this aligns with my discussion with Tris yesterday. I will make a note of open ended questions in my tracker as well.

SujitDey2022 commented 2 months ago

@KatyTaylor added an additional read length condition based on feedback from Tris.

BenTopping commented 2 months ago

Work breakdown

  1. (size: S) I can create batches of 2 for NovaSeqX

    • We can change the pipeline min_size in the record loader and manually change this value in production via the rails console. This will allow batches of size 2-8 to be created.
    • We can't limit NovaSeqX to exactly batches of 2 or 8 lanes without further work described in work breakdown 3.
    • This work will prevent tickets to psd-help for overriding existing validation.
  2. (size: S) If I select requests where the requested read lengths are different, I am only permitted to create a batch for the same read lengths.

    • This will be similar to DPL-783 PR
    • Q: Is this for all pipelines or just NovaSeqX?
  3. (size: L) Fine-grained batch creation based off request or other data.

    • This will allow us to add additional validation to batch creation other than just min/max batch size.
    • This work can cover NovaSeqX initially but should be setup in such a way that other pipelines can implement it.
    • When selecting a request in a batch, check the pipeline validation parameters and grey out subsequent invalid requests. e.g. in this case selecting a 1.5B request greys out 10B requests. Or validation should be completed on 'Create batch' and display an error message although greying out is likely preferred.
    • This is a large story because it will require a new method of batch validation and changes to config driven pipeline batch creation behaviour.
    • This will set up but not complete DPL-700.

Other notes Acceptance criteria: I cannot create a batch containing requests of different types of flow cells types(10B, 25B, 1.5B). Already completed by DPL-783

seenanair commented 3 days ago

Following the discussion with @BenTopping , @KatyTaylor and @dasunpubudumal the scope of the story is defined as follows

Batch creation validation in NovaSeq X:   When selecting requests from the NovaSeq X pipeline inbox (Pipelines tab > NovaSeqX PE):

This story is dependendant on Y24-121 and this can be implemented only after the completion of it.

SujitDey2022 commented 2 days ago

Hi @seenanair, I was looking your comment above and noticed that the story that you mentioned as a pre-requisite DPL-783 is already completed. Is the referenced story number correct?

seenanair commented 2 days ago

Hi @seenanair, I was looking your comment above and noticed that the story that you mentioned as a pre-requisite DPL-783 is already completed. Is the referenced story number correct?

@SujitDey2022 Sorry, corrected it. The pre-requisite is Y24-121 which Dasun is currently doing.