Speed up and extend normalizations

[mbuttner]

Current normalization tools in pytometry use the FlowKit implementation and are FlowJo input compatible, but could use a speed up, especially the functions for logicle and biexponential transformations. The Cytotransform package implements commonly used transformations and is much faster. An adaptation of these functions to the anndata format would be amazing.

[burtonrj]

Cytotransform uses the FastLogicle C++ implementation from https://www.ncbi.nlm.nih.gov/pmc/articles/PMC4761345/. How do you feel about making cytotransform a dependency of pytometry? Might be easier to manage rather than ingesting the C++ code into this repo?

It also accepts a Pandas DataFrame as input so a wrapper for anndata should be easy to create. I'm happy to pick this one up.

[mbuttner]

I'm happy to make CytoTransform a dependency of pytometry. The wrapper should be relatively easy because most data matrices for flow data are dense. My only concern is that the pandas DataFrame is less memory efficient than a numpy 2D matrix, which might become problematic as soon as you use 10M+ cells. So I think that using the 2D numpy matrix as input would be preferable, especially when the C++ implementation will convert the input to a 2D array anyways.

[burtonrj]

That makes perfect sense. It will be a simple refactor to cytotransform to fix this since under the hood it acts on the numpy array of the pandas dataframe anyway. I'll refactor cytotransform base class to accept a dataframe or numpy array and skip wrangling the output into a dataframe when numpy arrays are provided as input - see burtonrj/cytotransform#5

[whitews]

Hi Maren and Ross,

It's great to see the Python cytometry ecosystem growing. However, I'm concerned that we may be unnecessarily fracturing our efforts. It seems we have 3 different implementations of transformations here. Perhaps there is a way to work together?

Kind regards,
Scott

[mbuttner]

Hi Scott,
thanks for reaching out! The current normalization implementation in pytometry largely follow your implementation in the FlowKit package, and I respect your concern. Happy to talk and join forces.

Best,
Maren

[whitews]

Maren,

Most transformations in FlowKit are modeled after the GatingML 2.0 specification, and their implementation is defined in the FlowUtils package. The exception is the FlowJo biex transformation, as you've discovered. The history of FlowUtils comes from inheriting the original fcm code base, which I broke apart with the hope that the individual parts would be more useful to others.

I realize the requirements / needs for projects may differ, but the transformation functions are one of those rare, well-defined requirements that seem better served by many eyes on one set of implementations. I can move the FJ biex xform to FlowUtils if that would help. The reason I kept it separate was the added dependency of SciPy, where FlowUtils has the singular dependency of NumPy.

And, I see Ross has parallelized some of the other xforms in the CytoTransform package. I also thought about doing this as the parallelization for transforming FCM events is obvious, but multiprocessing (mp) in Python has a double-edge. Nesting mp in Python is "here be dragons" territory. I use mp in FlowKit for processing multiple FCS samples using the same gating strategy, with the rationale that a user would unlikely mp different gating strategies. Adding mp to the transformations that are necessary for the gating functions threw a wrench in the works. Still, it is an interesting idea Ross has for making the xform classes optionally mp.

Enough rambling by me, curious what you both think and what ideas you have.

-Scott

[burtonrj]

Hi guys!

Thanks for the input Scott! Much appreciated and I too agree transformations is an area where we should all be aligned regardless of what else we are doing - maybe also on things like compensations, various utilities, gate definitions, but we can discuss that another time - then we should allow the user some choice around what frameworks they want to work in. Maren is doing great work expanding cytometry into the scanpy ecosystem and I am right behind her on that! In fact I even hope to retire CytoPy and point people this direction.

As for cytotransform, I took inspiration from your work Scott and tried to make it GatingML 2.0 compatible and also performing testing against the GatingML 2.0 definitions (https://github.com/burtonrj/cytotransform/blob/main/tests/test_transformer.py). The mp approach is an ambitious attempt to brute force transforms on enormous datasets....but mp in Python is a swamp I will agree (hopefully to be fixed in 3.13 onwards https://peps.python.org/pep-0703/). Recognising that mp is not always desirable, cytotransform will just execute the function in a single process if n_jobs is 0 or 1. I've also made this the default behaviour if you have less than 10000 events....maybe it should just be the default behaviour anyway? MP should be an explicit choice? Interested to hear peoples thoughts on this.

[burtonrj]

Oh, also I've had far less installation woes with the FastLogicle.cpp implementation coupled with PyBind11. I haven't compared the performance of FastLogicle vs your C implementation though @whitews, not sure which is faster or more precise.

[mbuttner]

Hi there,

thank you all for your suggestions. I might be more taciturn, but I enjoy the discussion.

1. About multiprocessing:
   I had some good experience with multiprocessing in Python on our Linux cluster so far, but I do understand that this might not be generalizable to other architectures. So I agree with @whitews that we should disable multiprocessing for normalization by default and leave it to the user - as @burtonrj stated "make it an active choice".

2. Normalization functions as defined in FlowUtils/FlowKit:
   I agree that the definitions of the data transformation leaves no room for variation. I do realize that the current implementation of the arcsinh does not quite follow the GatingML 2.0 standard, so that should be adapted. When I looked into FlowKit, I realized that pytometry follows much less the object-oriented coding paradigm as FlowKit does. That is why I decided to adapt these transformations to fit with the anndata format and the scanpy coding style instead of relying on FlowUtils/FlowKit directly. That results into a parallel development, obviously, but I haven't figured out a more elegant way to do this (or at least I didn't when I first implemented the functions).
   Second, I extended the arcsinh and biExp transform functions such that every channel can be transformed individually.That's something that FlowJo does for the biExp function and which is a common practice for spectral flow data. My concern is that when we decide to rely more on FlowUtils, we might be less flexible to respond to user requests to extend the functionality (see issue #54). I'm leaning more towards keeping the pytometry package separate from FlowKit and follow through with @burtonrj's implementations in cytoTransform.

[whitews]

Maren & Ross,

It sounds like we are pretty much on the same page. What if I move the transform classes from FlowKit to FlowUtils, along with moving the FlowJo Biex implementation there. This adds SciPy as a dependency, which is why it was not done previously, but does put all the transforms in one place.

Ross, would you be available to help maintain FlowUtils if we did this? We could add the optional mp as well. I do feel that the more eyes we have on these critical implementations the better, inline with the Torvalds quote "given enough eyeballs, all bugs are shallow".

One other point is that the Sample class in FlowKit does allow for a dictionary of transforms mapped to individual channels. The Gate classes are also designed to be used independently from the heavy Session & Workspace classes. Not sure if it was clear, but FlowKit is more or less feature complete. It will never expand to have clustering algos, a GUI, etc. This was a decision made early on after being a bit stung by the algo chase building ReFlow.

[mbuttner]

Hi Scott,

that sounds good to me. It would be great if the FlowJo Biexp implementation would also be in FlowUtils. pytometry has the scipy dependency already, so that doesn't change much. I guess that I have to spend more time with FlowKit to fully understand its functionality and implementation - thank you for pointing this out 🤷‍♀️
@burtonrj you'll be making the call as this decision will affect you the most. If yes, this issue turns into "inherit FlowUtils normalizations".

[burtonrj]

Hi all,

I'm happy with this call, I was going to have to maintain cytotransform anyway so it makes no difference to me, other than I won't be maintaining it on my own - completely agree with you @whitews, the more eyes the better!

Let me know next steps RE changes to FlowUtils.

[whitews]

Great, I've started a discussion on the FlowUtils repo here:
whitews/FlowUtils#11

We can plan the changes there (and avoid spamming this issue :)