add method of "group_mz_int" by MuyaoXi9271 · Pull Request #382 · rformassspectrometry/Spectra

MuyaoXi9271 · 2026-03-31T08:45:12Z

No description provided.

Merge branch 'devel' of https://github.com/MuyaoXi9271/Spectra_MX into devel # Conflicts: # R/peak-list-functions.R

…original package

jorainer · 2026-03-31T10:32:11Z

@MuyaoXi9271 , can you please use 4 whitespaces (not 2)? otherwise also lines that you did not edit or change look as modified.

this setting should actually be defined in the .editorconfig file - and ideally, your editor/IDE should read this file and set the configuration accordingly.

jorainer · 2026-04-08T08:00:03Z

Dear @MuyaoXi9271 , can you please also describe what the goal/intention of your updates are? Maybe also have a look at the reduceSpectra() function which would report for groups of peaks with similar m/z the one with the highest intensity (for each spectrum)?

MuyaoXi9271 · 2026-04-10T09:41:32Z

Dear @MuyaoXi9271 , can you please also describe what the goal/intention of your updates are? Maybe also have a look at the reduceSpectra() function which would report for groups of peaks with similar m/z the one with the highest intensity (for each spectrum)?

Dear @jorainer,

Thanks for your suggestion.

The DDA data acquired from the Bruker instrument from my lab is that the same precursor has several spectra at each collision energy (in total 5 ), so I managed to get a consensus spectrum for each precursor at each collision energy. When I use the existing group function within Spectra::combineSpectra like
Spectra::combineSpectra(ex_spectra_filt, FUN = combinePeaksData, tolerance = 0.005, group_method = "group", peaks = intersect, minProp = 0.7, intensityFun = max, mzFun = median, f = ex_spectra_filt$collisionEnergy)_
I noticed that the difference in mass between smallest and largest within the same mass group is larger than the required threshold, so I include the condition of intensity to restrict how to group the masses, then to get the consensus spectrum for each precursor at each collision energy.

Best,
Muyao

jorainer · 2026-04-10T10:11:00Z

OK, so the group_mz_int() is then doing a different (less greedy) grouping than group() that actually ensures the grouped peaks are all within tolerance and ppm? but this function is not included in this PR, right?

an additional question:

I guess the problem is that the reported (consensus) m/z might be off when you report the median m/z of the peak group - but with the intensity weighted mean it should be somewhat similar to your result? because then the m/z closer to the max intensity peak will be reported.

MuyaoXi9271 · 2026-04-10T13:51:36Z

OK, so the group_mz_int() is then doing a different (less greedy) grouping than group() that actually ensures the grouped peaks are all within tolerance and ppm? but this function is not included in this PR, right?

an additional question:

I guess the problem is that the reported (consensus) m/z might be off when you report the median m/z of the peak group - but with the intensity weighted mean it should be somewhat similar to your result? because then the m/z closer to the max intensity peak will be reported.

I would say a little modification on group() to make sure the grouped peaks are all within tolerance and ppm. The function group is from the package MsCoreUtils, so I added the function group_mz_int to that package as well.

The idea of the function group_mz_int is to order the intensity of all masses from several spectra for the same precursor at the same collision energy, decreasingly, then the mass with the highest intensity was picked up, and then other similar masses are found within a threshold. This step was repeated until picking the last mass group. In theory, the m/z closer to the max intensity from each mass group should be reported .

MuyaoXi9271 · 2026-04-13T07:24:09Z

Hi @jorainer , should I open a "pull request" under the package of MsCoreUtils?

jorainer · 2026-04-14T09:50:53Z

OK, thanks for explaining. I think I understood it. And yes @MuyaoXi9271 , please make a PR to MsCoreUtils with your group_mz_int() function. Ideally, the function should then have the same result type than group (i.e. an integer with the assignment of the values to each group).

Then we can think how to also modify the existing combinePeaks() function to allow using different grouping functions (ideally without having to hard-code it using if/else or switch statements).

jorainer

Thanks for the PR. There are some changes and open questions that need to be addressed.

jorainer · 2026-06-18T08:13:03Z

 combinePeaksData <- function(x, intensityFun = base::mean,
                             mzFun = base::mean, weighted = FALSE,
                             tolerance = 0, ppm = 0, timeDomain = FALSE,
+                             group_method = c("group", "group_mz_int"),


Please use groupMethod instead of group_method to be consistent with the other parameter names. Also, please add the groupMethod parameter as last parameter (i.e., after minProp) to not cause problems if any user uses positional argument passing instead of named parameters.

jorainer · 2026-06-18T08:14:06Z

+                             group_method = c("group", "group_mz_int"),
                             peaks = c("union", "intersect"), main = integer(),
                             minProp = 0.5, ...) {
    peaks <- match.arg(peaks)


you should also do a groupMethod <- match.arg(groupMethod) here so that the input parameter values are checked and a single one is chosen (by default the first).

jorainer · 2026-06-18T08:25:11Z

    }
+
+    #New lines start-------- 
+    mzs <- mzs[mz_groups != -1]


these lines (mzs <- mzs[mz_groups != -1] to mz_groups <- mz_groups[mz_groups != -1]) should go above inside the else if (group_method == "group_mz_int"). this is not needed for the "group" method and will slow the function down.

jorainer · 2026-06-18T08:26:24Z

    ints <- split(ints, mz_groups)
+
+
+    # --- New lines start for keeping the mz within the tolerance ----------------------


why is this whole code (line 255 until 276) needed? this is what the group_mz_int() should actually do, no?

jorainer · 2026-06-18T08:29:49Z

    } else
        mzs <- vapply1d(mzs, FUN  = mzFun)
+
+    #New lines start ---------------------------------------------


why are these changes here needed? mzs and ints where already ordered by m/z before, so why should they be re-ordered?

jorainer · 2026-06-18T08:31:21Z

@@ -0,0 +1,3 @@
+Optimization of function of "peak-list-function.R" from lin 231 to line 252


please remove the Update.txt file.

MuyaoXi9271 and others added 7 commits January 26, 2023 15:25

modify combinePeaks to keep consistent as function of group_mz_int

db4755d

Make mzs increasingly after add new function in MscoreUtils

03df1dc

Merge branch 'rformassspectrometry:master' into master

e0e647e

update_20231106

3a31c33

Merge branch 'devel' of https://github.com/MuyaoXi9271/Spectra_MX into devel # Conflicts: # R/peak-list-functions.R

update

53d7684

change function name combinePeaks into combinePeaksData aligned with …

a45a669

…original package

Merge branch 'rformassspectrometry:main' into devel

fc31b95

Muyao Xi and others added 2 commits April 7, 2026 10:21

indentation

a277c3a

Merge branch 'main' into devel

517d1c3

Merge branch 'rformassspectrometry:main' into devel

89531e3

MuyaoXi9271 mentioned this pull request Apr 19, 2026

Add one more function "group_mz_int" to group.R rformassspectrometry/MsCoreUtils#148

Merged

Merge branch 'rformassspectrometry:main' into devel

eb629ba

jorainer requested changes Jun 18, 2026

View reviewed changes

Merge branch 'rformassspectrometry:main' into devel

2888e7c

		ints <- split(ints, mz_groups)


		# --- New lines start for keeping the mz within the tolerance ----------------------

		@@ -0,0 +1,3 @@
		Optimization of function of "peak-list-function.R" from lin 231 to line 252

Conversation

MuyaoXi9271 commented Mar 31, 2026

Uh oh!

jorainer commented Mar 31, 2026

Uh oh!

jorainer commented Apr 8, 2026

Uh oh!

MuyaoXi9271 commented Apr 10, 2026

Uh oh!

jorainer commented Apr 10, 2026

Uh oh!

MuyaoXi9271 commented Apr 10, 2026

Uh oh!

MuyaoXi9271 commented Apr 13, 2026

Uh oh!

jorainer commented Apr 14, 2026

Uh oh!

jorainer left a comment

Choose a reason for hiding this comment

Uh oh!

jorainer Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

jorainer Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

jorainer Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

jorainer Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

jorainer Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

jorainer Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants