Integration of CDK ExhaustiveFragmenter by JonasSchaub · Pull Request #133 · FelixBaensch/MORTAR

JonasSchaub · 2024-10-07T08:39:45Z

No description provided.

JonasSchaub · 2024-10-23T12:00:18Z

@ToLeWeiss , I fear performance will be a big issue here. I just started a fragmentation with 1000 natural products and it is still running. We need to discuss this, test it more, and maybe document it somewhere.

Final result of my run:

Oct 23, 2024 1:45:35 PM de.unijena.cheminf.mortar.controller.MainViewController lambda$importMoleculeFile$25
INFO: Successfully imported 1000 molecules from file: Picked_COCONUT_subset_1000_mols.sdf; 0 molecules could not be parsed into the internal data model (SMILES code generation failed). See above how many molecules could not be read from the input file at all or produced exceptions while preprocessing.
Oct 23, 2024 1:45:41 PM de.unijena.cheminf.mortar.controller.MainViewController startFragmentation
INFO: Start of method startFragmentation
Oct 23, 2024 1:45:41 PM de.unijena.cheminf.mortar.model.fragmentation.FragmentationService startFragmentation
INFO: Fragmentation "Exhaustive Fragmenter" (Exhaustive Fragmenter) starting. Current memory consumption: 168 MB
Oct 23, 2024 2:23:38 PM de.unijena.cheminf.mortar.model.fragmentation.FragmentationService startFragmentation
INFO: Fragmentation "Exhaustive Fragmenter" (Exhaustive Fragmenter) of 1,000 molecules complete. It took 2,277,137 ms. Current memory consumption: 2,496 MB
Oct 23, 2024 2:23:38 PM de.unijena.cheminf.mortar.model.fragmentation.FragmentationService startSingleFragmentation
INFO: Number of different fragments extracted: 169,490
Oct 23, 2024 2:23:39 PM de.unijena.cheminf.mortar.controller.MainViewController lambda$startFragmentation$44
INFO: End of method startFragmentation after 2277.6163057

And afterwards, it aborted with an out of memory error...
But as you can see, 169,490 fragments were generated from 1,000 natural products in 38 minutes (4 parallel threads and while I was also working on other things on the same laptop).

JonasSchaub

Looks good overall, just some minor things in the comments.

JonasSchaub · 2024-10-23T12:57:45Z

When the new fragmenter is added, we also need to describe it in the tutorial: https://docs.google.com/document/d/1dbpSZfdmOYaQqdYTDZj0NfKuy9TMp6T4SF6lCBB7bFo/edit?usp=sharing
@ToLeWeiss could you suggest some edits there? I need to redo some of the pictures, so that they are consistent.

JonasSchaub · 2024-11-07T09:53:06Z

@ToLeWeiss ready for re-review?

ToLeWeiss · 2024-11-07T10:17:29Z

Now it is ready for a review.

JonasSchaub

Minor comments

JonasSchaub · 2024-11-11T11:40:29Z

- * from the CDK, available for MORTAR.
+ * from the CDK available for MORTAR. It has a performance of O(n!) where n is the number of splittable bonds. Splittable
+ * bonds are defined as non-ring, single bonds that are connected to at least one other atom, that is <b>not</b> an
+ * implicit hydrogen.


Please use the term "terminal" here

JonasSchaub · 2024-11-11T11:49:49Z

@ToLeWeiss everything looks very good and ready to merge but based on what we just discussed, I would not merge it now, so that you can do your re-implementation of the exhaustive fragmentation here. What do you think?

ToLeWeiss · 2024-11-12T11:39:58Z

Do you mean a side by side implementation of the CDK Exhaustive Fragmenter and a "fast" Exhaustive Fragmenter approach here @JonasSchaub ?

JonasSchaub · 2024-11-12T14:09:12Z

Do you mean a side by side implementation of the CDK Exhaustive Fragmenter and a "fast" Exhaustive Fragmenter approach here @JonasSchaub ?

I mean that it probably won't make sense to integrate the CDK ExhaustiveFragmenter right now, in the state that it is in, but rather wait for your optimised implementation.

ToLeWeiss · 2024-11-12T15:32:45Z

Ah okay.
Yes that seems reasonable.

JonasSchaub · 2024-11-14T13:20:45Z

@ToLeWeiss please merge the production branch into your current branch when you have time (tell me if you need help with that).

ToLeWeiss · 2024-11-14T15:15:53Z

I would be happy to merge production into this branch to be up to date, but which strategy (normal merge or rebase) is preferred here @JonasSchaub ?

JonasSchaub · 2024-11-14T15:35:36Z

I would be happy to merge production into this branch to be up to date, but which strategy (normal merge or rebase) is preferred here @JonasSchaub ?

A normal merge would be the way to go (less dangerous than a rebase).

sonarqubecloud · 2024-11-14T18:01:34Z

Quality Gate failed

Failed conditions
10.6% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

…fragmentation utility of the CDK, as a fragmentation option rebased on the production branch to be up to date

… of the minimal fragment size of the exhaustive fragmenter

…f the exhaustive fragmentation and added the respective test class

…ue to automatic saturation

…xhaustive fragmenter class

…tion

…nfigurations for the SmilesGenerator

…n settings would propably be more appropriate

…or as multiple boolean values

…ented unique SMILES as a default setting

… addded a test and this statements

sonarqubecloud · 2026-03-16T15:19:26Z

Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
2.1% Duplication on New Code

See analysis details on SonarQube Cloud

ToLeWeiss · 2026-03-16T15:35:12Z

Hey @JonasSchaub the code would be ready for a review if you happen to find some time on your hands. (It seems i can not request a review because we probably are both authors)

Copilot

Pull request overview

Adds a new fragmentation algorithm integration by wrapping CDK’s ExhaustiveFragmenter, exposing its settings to the MORTAR GUI/settings system, and registering it in the fragmentation service.

Changes:

Introduces CDKExhaustiveFragmenter implementing IMoleculeFragmenter (settings, filtering/preprocessing, fragmentation).
Registers the new fragmenter in FragmentationService and adds en-GB UI strings for its settings.
Adds JUnit tests for basic instantiation and sample fragmentations (incl. stereo preprocessing behavior).

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`src/main/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/CDKExhaustiveFragmenter.java`	New wrapper around CDK exhaustive fragmentation with GUI-exposed settings and preprocessing/filter logic.
`src/main/java/de/unijena/cheminf/mortar/model/fragmentation/FragmentationService.java`	Registers the new fragmenter instance in the service’s fragmenter list.
`src/main/resources/de/unijena/cheminf/mortar/message/Message_en_GB.properties`	Adds display names/tooltips for the new fragmenter’s settings.
`src/test/java/de/unijena/cheminf/mortar/model/fragmentation/algorithm/CDKExhaustiveFragmenterTest.java`	Adds unit tests for basic settings access, fragmentation, and stereo preprocessing.
`gradlew`	Adds the Gradle POSIX wrapper script.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

JonasSchaub

It already looks quite good. You have to clean up some names, some doc, and some parameter inconsistencies, though. My "biggest" point is to add another setting that dis- and enables the a priori filtering by nr. of splittable bonds.

JonasSchaub · 2026-03-18T16:53:42Z

+CDKExhaustiveFragmenter.splittableBondsThreshold.displayName = Splittable Bonds threshold setting
+CDKExhaustiveFragmenter.minFragmentSize.tooltip = Defines the minimum number of atoms (inclusive) that are required for one Fragment. The number of atoms consists of all atoms, that are connected by more than one single bond or have more than one single bond.
+CDKExhaustiveFragmenter.minFragmentSize.displayName = Minimum fragment size setting
+CDKExhaustiveFragmenter.inclusiveMaxTreeDepth.tooltip = Defines the maximum depth of the fragment tree. This setting controls how deeply the fragmentation algorithm will explore molecular fragment combinations. A higher value allows for more extensive fragmentation but may increase computational complexity.


needs to be shortened a bit. And it definitely does increase comp. complexity!

sonarqubecloud · 2026-04-15T23:04:58Z

Quality Gate failed

Failed conditions
4.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

JonasSchaub added the enhancement New feature or request label Oct 7, 2024

JonasSchaub changed the title ~~Initialization of new fragmenter class~~ Integration of CDK ExhaustiveFragmenter Oct 7, 2024

ToLeWeiss marked this pull request as ready for review October 22, 2024 15:18

JonasSchaub commented Oct 23, 2024

View reviewed changes

JonasSchaub commented Nov 11, 2024

View reviewed changes

JonasSchaub added this to the MORTAR v2.0.0 milestone Nov 21, 2024

JonasSchaub assigned ToLeWeiss Nov 21, 2024

JonasSchaub and others added 11 commits February 4, 2026 15:44

Initialization of new fragmenter class

6d85ed7

work in progress: first rudimentary implementation of the exhaustive …

6a0aeca

…fragmentation utility of the CDK, as a fragmentation option rebased on the production branch to be up to date

work in progress: added hydrogen saturation and corrected the setting…

1534b27

… of the minimal fragment size of the exhaustive fragmenter

work in progress: functional implementation, except the saturation, o…

f8419dd

…f the exhaustive fragmentation and added the respective test class

work in progress: fixed formatting error and modified fragmentation d…

e989be8

…ue to automatic saturation

work in progress: removed the saturation option

c06bdd5

finalized implementation

97c69ed

small refactor for consistency with other fragmenters

2ee53c5

minor fixes

8cb96e6

applied requested fixes and added some further documentation to the e…

9dba94d

…xhaustive fragmenter class

correction for the performance and added further information

828aebb

ToLeWeiss added 2 commits February 4, 2026 16:13

fixed wrong comment and refined the class description

15fc053

wip: adapting to the new CDK exhaustive fragmenter

5af8971

ToLeWeiss force-pushed the dev-tom-exh-frag branch from 4eb3a6a to 5af8971 Compare February 4, 2026 16:36

ToLeWeiss added 11 commits February 13, 2026 09:33

wip: added some of the new options of the cdk implementation

87810fc

wip: added some of the new options and defaults of the cdk implementa…

c38409a

…tion

wip: added some settings but still unsure how to add the different co…

b791965

…nfigurations for the SmilesGenerator

wip: tried an enum variant for the smiles setting but multiple boolea…

c8e3c2a

…n settings would propably be more appropriate

wip: first attempt at implementing the variants of the smiles generat…

70a4642

…or as multiple boolean values

wip: removed the settings of molecule weights and CXSMILES and implem…

248caca

…ented unique SMILES as a default setting

added a splittable bond threshold that is used for filtering and also…

50eb55a

… addded a test and this statements

fixed the propagation of the settings to the actual fragmenter

96f482f

removed the setting of aromatic symbols

f1873c2

fixed a few minor things

d8f9b20

improved consistency in the comments

cd714af

JonasSchaub requested a review from Copilot March 18, 2026 14:50

Copilot started reviewing on behalf of JonasSchaub March 18, 2026 14:50 View session

Copilot AI reviewed Mar 18, 2026

View reviewed changes

JonasSchaub commented Mar 18, 2026

View reviewed changes

wip: implemented major part of the fixes

cbba105

Conversation

JonasSchaub commented Oct 7, 2024

Uh oh!

JonasSchaub commented Oct 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JonasSchaub left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JonasSchaub commented Oct 23, 2024

Uh oh!

JonasSchaub commented Nov 7, 2024

Uh oh!

ToLeWeiss commented Nov 7, 2024

Uh oh!

JonasSchaub left a comment

Choose a reason for hiding this comment

Uh oh!

JonasSchaub Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JonasSchaub commented Nov 11, 2024

Uh oh!

ToLeWeiss commented Nov 12, 2024

Uh oh!

JonasSchaub commented Nov 12, 2024

Uh oh!

ToLeWeiss commented Nov 12, 2024

Uh oh!

JonasSchaub commented Nov 14, 2024

Uh oh!

ToLeWeiss commented Nov 14, 2024

Uh oh!

JonasSchaub commented Nov 14, 2024

Uh oh!

sonarqubecloud bot commented Nov 14, 2024

Quality Gate failed

Uh oh!

sonarqubecloud bot commented Mar 16, 2026

Quality Gate passed

Uh oh!

ToLeWeiss commented Mar 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JonasSchaub left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JonasSchaub Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JonasSchaub commented Oct 23, 2024 •

edited

Loading