Surely that line output.getAtom( atom.getAtomicNumber() -1 )
Was meant to use output.getAtomCount() -1
To get the last atom added?
On Mon, Sep 5, 2022, 6:44 AM John Mayfield <john.wilkinson...@gmail.com>
wrote:
> Yes and it's correct, your code is adding bonds to atoms which don't exist
> yet! You need to add the atoms of the bond before the bond - 0..k not k ..
> n.
>
> As an aside the Mappings API has this method
> already (toSubstructureStream()) if you're coming via CDK substructure
> Pattern (although it doesn't do the radials/lone pairs... possible but I
> never found a use to have them explicitly like that).
>
> In general this code is an inefficient way to build the container... I
> think it's O(N^3) - but perhaps only O(N^2) with AtomContainer2 :-). Much
> better to add all the atoms, record these in a set/map, then loop all the
> bonds. Optionally for best performance you should do this "resync"
> operation where you map the source => target AtomRef. Maybe we should add
> this to the ACmanipulator, I thought there was something similar
> already though.
>
> John
>
> public static IAtomContainer extractSubstructure(IAtomContainer source,
> List<IAtom> atoms) {
> IAtomContainer output = source.getBuilder().newAtomContainer();
> Map<IAtom,IAtom> remap = new HashMap<>();
> for (IAtom atom : atoms) {
> output.addAtom(atom);
> source.getConnectedLonePairsList(atom).forEach(output::addLonePair);
>
> source.getConnectedSingleElectronsList(atom).forEach(output::addSingleElectron);
> // resync: get the AtomRef in the context of the new container. This
> // presumes atoms gets added at last position which is currently
> // always the case
> remap.put(atom, output.getAtom(atom.getAtomicNumber() - 1));
> }
> for (IBond bond : source.bonds()) {
> IAtom beg = remap.get(bond.getBegin());
> IAtom end = remap.get(bond.getEnd());
> if (beg != null && end != null) {
> output.addBond(bond);
>
> // or the more efficient but you get a "NEW" bond so may need to
> some fudging with
> // setting aromatcity/ring membership etc, however if you're
> selecting a substructure
> // you MUST recalculate these anyways.
> // output.addBond(beg.getIndex(), end.getIndex(),
> bond.getOrder(), bond.getStereo());
> }
> }
> return output;
> }
>
>
> On Mon, 5 Sept 2022 at 11:10, Uli Fechner <u...@pending.ai> wrote:
>
>>
>> Hi,
>>
>> I get a NoSuchAtomException when executing the following code (that I
>> inherited):
>>
>> public static IAtomContainer extractSubstructure(IAtomContainer source,
>> List<IAtom> atoms) {
>> IAtomContainer output =
>> SilentChemObjectBuilder.getInstance().newAtomContainer();
>> int k = 0;
>> for (IAtom atom : atoms) {
>> output.addAtom(atom);
>> source.getConnectedLonePairsList(atom).forEach(lp ->
>> output.addLonePair(lp));
>> source.getConnectedSingleElectronsList(atom).forEach(se ->
>> output.addSingleElectron(se));
>> k++;
>> for (int i = k; i < atoms.size(); i++) {
>> IBond bond = source.getBond(atom, atoms.get(i));
>> if (bond != null) {
>> output.addBond(bond);
>> }
>> }
>> }
>> return output;
>> }
>>
>> The stack trace starts at the line with
>> output.addBond(bond);
>>
>> and continues with the following lines:
>>
>> org.openscience.cdk.exception.NoSuchAtomException: Atom is not a member of
>> this AtomContainer
>> at
>> app//org.openscience.cdk.silent.AtomContainer2.getAtomRef(AtomContainer2.java:185)
>> at
>> app//org.openscience.cdk.silent.AtomContainer2.newBondRef(AtomContainer2.java:221)
>> at
>> app//org.openscience.cdk.silent.AtomContainer2.addBond(AtomContainer2.java:908)
>>
>> The exception *isn't* thrown if I replace the instantiation of the
>> IAtomContainer output with
>> IAtomContainer output = new AtomContainer();
>> I understand that the exception has something to do with the way
>> AtomContainer2 is implemented, but I don't know how to switch to the
>> AtomContainer2 implementation and still retain the intent of the code -
>> which is to only add atoms, their connected LPs, their connected single
>> electrons and their connected bonds, but not connected atoms - to the
>> returned subgraph. Best
>> Uli
>>
>>
>> _______________________________________________
>> Cdk-user mailing list
>> Cdk-user@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/cdk-user
>>
> _______________________________________________
> Cdk-user mailing list
> Cdk-user@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/cdk-user
>
_______________________________________________
Cdk-user mailing list
Cdk-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/cdk-user
