[Cdk-devel] [ cdk-Patches-2905749 ] AtomContainerManipulator Functions

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 454-5900

Patches item #2905749, was opened at 2009-11-29 21:22
Message generated for change (Comment added) made by asadrahman
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=320024&aid=2905749&group_id=20024

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Asad (asadrahman)
Assigned to: Nobody/Anonymous (nobody)
Summary: AtomContainerManipulator Functions

Initial Comment:
I have written few extended functions for SMSD. I was wondering if these functions could be incorporated with the present AtomContainerManipulator.

Thanks

Asad

----------------------------------------------------------------------

Comment By: Asad (asadrahman)
Date: 2009-11-30 00:30

Message:
I am not sure about the latest changes in the CDK AtomContainer clone
method as I wrote deepcopy method because the clone atom container used to
miss the atom IDs (i.e atom.getID use to be null) and some flags after
using AtomContainer.clone .

If flags are set (copied/cloned) and atom IDs are conserved then I would
think my problem is solved!

----------------------------------------------------------------------

Comment By: Egon Willighagen (egonw)
Date: 2009-11-30 00:18

Message:
I'd say that JavaDoc confirms that the method is supposed to make a deep
clone.

Flags, ID should be copied; that should be explored, and sounds like a
bug. 

Notification listeners should not be copied, I think.

Properties is just tricky, and no one ever tried to make an point about
that... one problem is that properties may or may not be clonable, and it
is typically impossible to deep clone them at all... so, currently
properties are not cloned in ChemObject.clone()... this is certainly
debateble... it has just never been discussed before.

This could certainly be something that we can merge into mainline CDK... I
would like to invite you to bring up your use case of cloning properties to
the cdk-user mailing list, and see if we can come to a general solution...

----------------------------------------------------------------------

Comment By: Asad (asadrahman)
Date: 2009-11-30 00:07

Message:
 AtomContainer.clone() has this comment attached to it
/**
     * Clones this AtomContainer object and its content.
     *
     * @return    The cloned object
     * @see       #shallowCopy
     */
Secondly

It doesn't set these functions

        newAtomContainer.setProperties(container.getProperties());
        newAtomContainer.setFlags(container.getFlags());
        newAtomContainer.setID(container.getID());
        newAtomContainer.notifyChanged();

----------------------------------------------------------------------

Comment By: Egon Willighagen (egonw)
Date: 2009-11-29 23:44

Message:
> makeDeepCopy make a deep copy of bonds and atoms too whereas
> AtomContainer.clone() is a shallow copy

Oh... but that can't be right... I'm pretty sure all IChemObject.clone()
methods are supposed to make deep clones...

----------------------------------------------------------------------

Comment By: Asad (asadrahman)
Date: 2009-11-29 23:31

Message:
1) Yes I have assigned this as patch too
2) makeDeepCopy make a deep copy of bonds and atoms too whereas
AtomContainer.clone() is a shallow copy.
3) I would really appreciate if few more functions can be added to
Standardizer class from another attached class MoleculeSanityCheck.java. I
am attaching this class too.

public class MoleculeSanityCheck {

    /**
     * 
     * @param molecule
     * @return
     */
    public static IAtomContainer checkAndCleanMolecule(IAtomContainer
molecule) {
        boolean isMarkush = false;
        for (IAtom atom : molecule.atoms()) {
            if (atom.getSymbol().equals("R")) {
                isMarkush = true;
                break;
            }
        }

        if (isMarkush) {
            System.err.println("Skipping Markush structure for sanity
check");
        }

        // Check for salts and such
        if (!ConnectivityChecker.isConnected(molecule)) {
            // lets see if we have just two parts if so, we assume its a
salt and just work
            // on the larger part. Ideally we should have a check to
ensure that the smaller
            //  part is a metal/halogen etc.
            IMoleculeSet fragments =
ConnectivityChecker.partitionIntoMolecules(molecule);
            if (fragments.getMoleculeCount() > 2) {
                System.err.println("More than 2 components. Skipped");
            } else {
                IMolecule frag1 = fragments.getMolecule(0);
                IMolecule frag2 = fragments.getMolecule(1);
                if (frag1.getAtomCount() > frag2.getAtomCount()) {
                    molecule = frag1;
                } else {
                    molecule = frag2;
                }
            }
        }
        fixAromaticity(molecule);
        return molecule;
    }

    /**
     *
     * @param mol
     */
    public static void fixAromaticity(IAtomContainer mol) {
        // need to find rings and aromaticity again since added H's

        IRingSet ringSet = null;
        try {
            AllRingsFinder arf = new AllRingsFinder();
            ringSet = arf.findAllRings(mol);

            // SSSRFinder s = new SSSRFinder(mol);
            // srs = s.findEssentialRings();

        } catch (Exception e) {
            e.printStackTrace();
        }

        try {
            // figure out which atoms are in aromatic rings:

ExtAtomContainerManipulator.percieveAtomTypesAndConfigureAtoms(mol);
            CDKHueckelAromaticityDetector.detectAromaticity(mol);
            // figure out which rings are aromatic:
            RingSetManipulator.markAromaticRings(ringSet);
            // figure out which simple (non cycles) rings are aromatic:
            // HueckelAromaticityDetector.detectAromaticity(mol, srs);
        } catch (Exception e) {
            e.printStackTrace();
        }

        // only atoms in 6 membered rings are aromatic
        // determine largest ring that each atom is a part of

        for (int i = 0; i <= mol.getAtomCount() - 1; i++) {

            mol.getAtom(i).setFlag(CDKConstants.ISAROMATIC, false);

            jloop:
            for (int j = 0; j <= ringSet.getAtomContainerCount() - 1; j++)
{
                //logger.debug(i+"\t"+j);
                IRing ring = (IRing) ringSet.getAtomContainer(j);
                if (!ring.getFlag(CDKConstants.ISAROMATIC)) {
                    continue jloop;
                }

                boolean haveatom = ring.contains(mol.getAtom(i));

                //logger.debug("haveatom="+haveatom);

                if (haveatom && ring.getAtomCount() == 6) {
                    mol.getAtom(i).setFlag(CDKConstants.ISAROMATIC, true);
                }

            }

        }

    }
}
4) first one will fix  changes in nitros given by N(=O)(=O) to
[N+](=O)[O-] and second one will fix changes nitros given by [N+](=O)[O-]
to N(=O)(=O)  (these two function were present in CDK at some point)
5) aromatizeMolecule is same as CDKHueckelAromaticityDetector Except that
aromatizeMolecule depends  upon percieveAtomTypesAndConfigureAtoms of this
class. percieveAtomTypesAndConfigureAtoms handle NULL exception.

IAtomType matched = matcher.findMatchingAtomType(container, atom);
                if (matched != null) {
                    AtomTypeManipulator.configure(atom, matched);
                }

----------------------------------------------------------------------

Comment By: Rajarshi Guha (rajarshi)
Date: 2009-11-29 22:11

Message:
A few comments. 

1. Can you make this a patch - easier to apply and look at the differences
with the current class.

2. what does makeDeepCopy do that AtomContainer.clone() doesn't do? Can
any extra functionality in your method be added to AtomContainer.clone()

3. fix* look like they could be candidate methods for a Standardizer
class

4. What are differences between fixNitroGroups and fixNitroGroups2?

5. How does aromatizeMolecule differ from
CDKHueckelAromaticityDetector.perceiveAromaticity?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=320024&aid=2905749&group_id=20024