decsriptor names - a proposed refactoring

[Rajarshi Guha-3]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Currently, a GUI wrapper around the descriptor classes, requires that  
a descriptor sucessfully be evaluated to get the names of descriptor  
values.

The problem is that if a descriptor throws an exception, we can't get  
the names. As a result the column naming gets all messed up.

The solution to this requires one of two possible modifications

1) Provide a method in the IDescriptor interface called  
getDescriptorNames() that is independent of the calculate() method.  
So it would just imply moving the name generation code in  a  
descriptor to a method outside of calculate().

2) Descriptors never throw exceptions - rather if an exception is  
thrown, it is included as a field in DescriptorValue, with some form  
of null value to represent the uncalculated descriptors

My preference is 1), since 2) results in a different exception  
handling mechanism from the standard approach. Of course, 2) is  
easier to do.

In case 1),the name method would take into account descriptor  
parameters (if they affect the number of names to be generated), so  
in general, should be called after setting descriptor parameters if  
required.

- -------------------------------------------------------------------
Rajarshi Guha  <rguha@...>
GPG Fingerprint: D070 5427 CC5B 7938 929C  DD13 66A1 922C 51E7 9E84
- -------------------------------------------------------------------
A sine curve goes off to infinity, or at least the end
of the blackboard.
        -- Prof. Steiner


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (Darwin)

iEYEARECAAYFAkh+Jp0ACgkQZqGSLFHnnoT9ggCeJRj77mKoOasr7GaVhm6XIk4E
ylcAniaKdL0jz5ZEI3PoSjAwSCkiP/Rj
=23iP
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Cdk-devel mailing list
Cdk-devel@...
https://lists.sourceforge.net/lists/listinfo/cdk-devel

[Egon Willighagen-5]

On Wed, Jul 16, 2008 at 6:49 PM, Rajarshi Guha <rguha@...> wrote:
> The solution to this requires one of two possible modifications
>
> 1) Provide a method in the IDescriptor interface called
> getDescriptorNames() that is independent of the calculate() method.
> So it would just imply moving the name generation code in  a
> descriptor to a method outside of calculate().

And the calculate() method would simply reuse this method...

> 2) Descriptors never throw exceptions - rather if an exception is
> thrown, it is included as a field in DescriptorValue, with some form
> of null value to represent the uncalculated descriptors

I think we settled on returning NaN's, didn't we? If not, this is what
I would like to propose now as default behavior, instead of throwing
exceptions...

But we could still add the above getDescriptorNames() method... that's
useful for introspection prior to calculation...

> My preference is 1), since 2) results in a different exception
> handling mechanism from the standard approach. Of course, 2) is
> easier to do.

Yes, and ensures that descriptor matrices can be properly constructed...

> In case 1),the name method would take into account descriptor
> parameters (if they affect the number of names to be generated), so
> in general, should be called after setting descriptor parameters if
> required.

Indeed. If getDescriptorNames() is called prior to setting parameters,
it is based on the default parameters for that descriptor.

Egon

--
----
http://chem-bla-ics.blogspot.com/

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Cdk-devel mailing list
Cdk-devel@...
https://lists.sourceforge.net/lists/listinfo/cdk-devel

[Rajarshi Guha-3]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


On Jul 16, 2008, at 2:09 PM, Egon Willighagen wrote:

> On Wed, Jul 16, 2008 at 6:49 PM, Rajarshi Guha <rguha@...>  
> wrote:
>> The solution to this requires one of two possible modifications
>>
>> 1) Provide a method in the IDescriptor interface called
>> getDescriptorNames() that is independent of the calculate() method.
>> So it would just imply moving the name generation code in  a
>> descriptor to a method outside of calculate().
>
> And the calculate() method would simply reuse this method...

Correct - though given a separate method to get desc names, there is  
no real need to call it in calculate()

>> 2) Descriptors never throw exceptions - rather if an exception is
>> thrown, it is included as a field in DescriptorValue, with some form
>> of null value to represent the uncalculated descriptors
>
> I think we settled on returning NaN's, didn't we?

Hmm, we do indeed return NaN's

> If not, this is what
> I would like to propose now as default behavior, instead of throwing
> exceptions...

However sometimes exceptions are still throw. The reason why we need/
should throw exceptions (somehow) is so that the caller knows why the  
calculation failed. Currently, we throw an exception explicitly. An  
alternative is to update DescriptorValue so that it has a field  
containing the exception

> But we could still add the above getDescriptorNames() method... that's
> useful for introspection prior to calculation...

I agree

>> In case 1),the name method would take into account descriptor
>> parameters (if they affect the number of names to be generated), so
>> in general, should be called after setting descriptor parameters if
>> required.
>
> Indeed. If getDescriptorNames() is called prior to setting parameters,
> it is based on the default parameters for that descriptor.

Correct

- -------------------------------------------------------------------
Rajarshi Guha  <rguha@...>
GPG Fingerprint: D070 5427 CC5B 7938 929C  DD13 66A1 922C 51E7 9E84
- -------------------------------------------------------------------
Paper or plastic?
Not 'Not paper AND not plastic!!'
          -- Augustus DeMorgan in a grocery store


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (Darwin)

iEYEARECAAYFAkh+OwEACgkQZqGSLFHnnoRe2wCfX1eOUKAP4fpBpsLGhRQDorjM
qM8AnjdmoKfpaqzzxFbcteWvXZwdHWcS
=IaQK
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Cdk-devel mailing list
Cdk-devel@...
https://lists.sourceforge.net/lists/listinfo/cdk-devel

[Egon Willighagen-5]

On Wed, Jul 16, 2008 at 8:16 PM, Rajarshi Guha <rguha@...> wrote:
>>> 1) Provide a method in the IDescriptor interface called
>>> getDescriptorNames() that is independent of the calculate() method.
>>> So it would just imply moving the name generation code in  a
>>> descriptor to a method outside of calculate().
>>
>> And the calculate() method would simply reuse this method...
>
> Correct - though given a separate method to get desc names, there is no real
> need to call it in calculate()

I think the labels are part of the returned DescriptorValue...

>>> 2) Descriptors never throw exceptions - rather if an exception is
>>> thrown, it is included as a field in DescriptorValue, with some form
>>> of null value to represent the uncalculated descriptors
>>
>> I think we settled on returning NaN's, didn't we?
>
> Hmm, we do indeed return NaN's

Yes, an important reason is that it makes it much easier to get a nice
rectangular descriptor matrix...

>> If not, this is what
>> I would like to propose now as default behavior, instead of throwing
>> exceptions...
>
> However sometimes exceptions are still throw. The reason why we need/should
> throw exceptions (somehow) is so that the caller knows why the calculation
> failed. Currently, we throw an exception explicitly. An alternative is to
> update DescriptorValue so that it has a field containing the exception

Good point! Let's extend the DescriptorValue API with a field to hold
an exception.

>> But we could still add the above getDescriptorNames() method... that's
>> useful for introspection prior to calculation...
>
> I agree

Good.

>>> In case 1),the name method would take into account descriptor
>>> parameters (if they affect the number of names to be generated), so
>>> in general, should be called after setting descriptor parameters if
>>> required.
>>
>> Indeed. If getDescriptorNames() is called prior to setting parameters,
>> it is based on the default parameters for that descriptor.
>
> Correct

OK, so the plan is:

1. add the getDescriptorNames() method to the IDescriptor interface
2. change the behavior of calculate() and add a test to
AbstractDescriptorTest (or whatever I called that thingy) should test
for calculate() methods never throwing an exception, but throwing some
bad input towards calculate()...

Egon

--
----
http://chem-bla-ics.blogspot.com/

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Cdk-devel mailing list
Cdk-devel@...
https://lists.sourceforge.net/lists/listinfo/cdk-devel

[Rajarshi Guha-3]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


On Jul 16, 2008, at 2:26 PM, Egon Willighagen wrote:

> On Wed, Jul 16, 2008 at 8:16 PM, Rajarshi Guha <rguha@...>  
> wrote:
>>>> 1) Provide a method in the IDescriptor interface called
>>>> getDescriptorNames() that is independent of the calculate() method.
>>>> So it would just imply moving the name generation code in  a
>>>> descriptor to a method outside of calculate().
>>>
>>> And the calculate() method would simply reuse this method...
>>
>> Correct - though given a separate method to get desc names, there  
>> is no real
>> need to call it in calculate()
>
> I think the labels are part of the returned DescriptorValue...

Aah,correct. Yes, that wouldn't change then

>>>
>>> If not, this is what
>>> I would like to propose now as default behavior, instead of throwing
>>> exceptions...
>>
>> However sometimes exceptions are still throw. The reason why we  
>> need/should
>> throw exceptions (somehow) is so that the caller knows why the  
>> calculation
>> failed. Currently, we throw an exception explicitly. An  
>> alternative is to
>> update DescriptorValue so that it has a field containing the  
>> exception
>
> Good point! Let's extend the DescriptorValue API with a field to hold
> an exception.

OK - so just to confirm - calculate() must *not* throw any exceptions

> OK, so the plan is:
>
> 1. add the getDescriptorNames() method to the IDescriptor interface
> 2. change the behavior of calculate() and add a test to
> AbstractDescriptorTest (or whatever I called that thingy) should test
> for calculate() methods never throwing an exception, but throwing some
> bad input towards calculate()...

3. Updated DescriptorValue to have a field of type Exception, which  
can be null

Do you want to do this? Or shall I start?

- -------------------------------------------------------------------
Rajarshi Guha  <rguha@...>
GPG Fingerprint: D070 5427 CC5B 7938 929C  DD13 66A1 922C 51E7 9E84
- -------------------------------------------------------------------
A motion to adjourn is always in order.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (Darwin)

iEYEARECAAYFAkh+RkoACgkQZqGSLFHnnoQ5CgCfXEQrTi8bwY4OzbU2/BLvLwJM
2bAAoOsjbX5A9v9qfRLHtANDE3xeWVr/
=UPvl
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Cdk-devel mailing list
Cdk-devel@...
https://lists.sourceforge.net/lists/listinfo/cdk-devel

[Egon Willighagen-5]

On Wed, Jul 16, 2008 at 9:04 PM, Rajarshi Guha <rguha@...> wrote:

>>> However sometimes exceptions are still throw. The reason why we
>>> need/should
>>> throw exceptions (somehow) is so that the caller knows why the
>>> calculation
>>> failed. Currently, we throw an exception explicitly. An alternative is to
>>> update DescriptorValue so that it has a field containing the exception
>>
>> Good point! Let's extend the DescriptorValue API with a field to hold
>> an exception.
>
> OK - so just to confirm - calculate() must *not* throw any exceptions

Agreed, but store it in a field in the DescriptorValue... so, see below for 3)

>> OK, so the plan is:
>>
>> 1. add the getDescriptorNames() method to the IDescriptor interface
>> 2. change the behavior of calculate() and add a test to
>> AbstractDescriptorTest (or whatever I called that thingy) should test
>> for calculate() methods never throwing an exception, but throwing some
>> bad input towards calculate()...
>
> 3. Updated DescriptorValue to have a field of type Exception, which can be
> null

Yes, forgot to add that :)

> Do you want to do this? Or shall I start?

Please go ahead. I'll monitor and comment where I feel I need to :)

Egon

--
----
http://chem-bla-ics.blogspot.com/

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Cdk-devel mailing list
Cdk-devel@...
https://lists.sourceforge.net/lists/listinfo/cdk-devel

[Rajarshi Guha-3]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


On Jul 16, 2008, at 3:07 PM, Egon Willighagen wrote:

>>>
>>> OK, so the plan is:
>>>
>>> 1. add the getDescriptorNames() method to the IDescriptor interface
>>> 2. change the behavior of calculate() and add a test to
>>> AbstractDescriptorTest (or whatever I called that thingy) should  
>>> test
>>> for calculate() methods never throwing an exception, but throwing  
>>> some
>>> bad input towards calculate()...
>>
>> 3. Updated DescriptorValue to have a field of type Exception,  
>> which can be
>> null
>
> Yes, forgot to add that :)
>
>> Do you want to do this? Or shall I start?
>
> Please go ahead. I'll monitor and comment where I feel I need to :)

It's being done in the branch named rajasrshi-descnames

- -------------------------------------------------------------------
Rajarshi Guha  <rguha@...>
GPG Fingerprint: D070 5427 CC5B 7938 929C  DD13 66A1 922C 51E7 9E84
- -------------------------------------------------------------------
Bus error -- driver executed.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (Darwin)

iEYEARECAAYFAkh+vmkACgkQZqGSLFHnnoQVqgCgtAfHSQmmDxu7ci5j5Z80tqSP
rjYAoLHmJdzcbU72ED7lbAc4hIPd8cnS
=5/2C
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Cdk-devel mailing list
Cdk-devel@...
https://lists.sourceforge.net/lists/listinfo/cdk-devel

[Rajarshi Guha-3]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


On Jul 16, 2008, at 3:07 PM, Egon Willighagen wrote:

>>> 1. add the getDescriptorNames() method to the IDescriptor interfac

For  non-molecular descriptors what naming scheme is meant to be  
used? It looks like, nobody ever updated the code for atomic, bond,  
atompair descs to provide names!

At this point, I'm leaning towards arbitrary names - the question is,  
should the atom or bond in question be somehow indicated in the name?

Out of curiosity, does anybody use these descriptors?

- -------------------------------------------------------------------
Rajarshi Guha  <rguha@...>
GPG Fingerprint: D070 5427 CC5B 7938 929C  DD13 66A1 922C 51E7 9E84
- -------------------------------------------------------------------
C Code.
C Code Run.
Run, Code, RUN!
        PLEASE!!!!


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (Darwin)

iEYEARECAAYFAkh+zcEACgkQZqGSLFHnnoSe1ACgoZF0CxhY6aiWuMj3okHr8iT7
c2AAn0Msed3J9H9NSak5pmPfl6VNkzF1
=+o9n
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Cdk-devel mailing list
Cdk-devel@...
https://lists.sourceforge.net/lists/listinfo/cdk-devel

[Egon Willighagen-5]

On Thu, Jul 17, 2008 at 6:42 AM, Rajarshi Guha <rguha@...> wrote:
> Out of curiosity, does anybody use these descriptors?

At least the RDF descriptors have been used for NMR shift prediction...

Egon

--
----
http://chem-bla-ics.blogspot.com/

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Cdk-devel mailing list
Cdk-devel@...
https://lists.sourceforge.net/lists/listinfo/cdk-devel

[Rajarshi Guha-3]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


On Jul 17, 2008, at 12:42 AM, Rajarshi Guha wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
> On Jul 16, 2008, at 3:07 PM, Egon Willighagen wrote:
>
>>>> 1. add the getDescriptorNames() method to the IDescriptor interfac
>
> For  non-molecular descriptors what naming scheme is meant to be
> used? It looks like, nobody ever updated the code for atomic, bond,
> atompair descs to provide names!

Aah, sorry. Names are there.

Off to sleep

- -------------------------------------------------------------------
Rajarshi Guha  <rguha@...>
GPG Fingerprint: D070 5427 CC5B 7938 929C  DD13 66A1 922C 51E7 9E84
- -------------------------------------------------------------------
There is no truth to the allegation that statisticians are mean.
They are just your standard normal deviates.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (Darwin)

iEYEARECAAYFAkh+12UACgkQZqGSLFHnnoSnZgCfV0r07MTUHxZPGGn0uS7iVVHk
e2QAoLvoP4troF8wm2AgxGF3gxUj1DCh
=HdBy
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Cdk-devel mailing list
Cdk-devel@...
https://lists.sourceforge.net/lists/listinfo/cdk-devel

[Miguel Rojas Cherto-3]

> On Jul 16, 2008, at 3:07 PM, Egon Willighagen wrote:
>
> >>> 1. add the getDescriptorNames() method to the IDescriptor interfac
>
> For  non-molecular descriptors what naming scheme is meant to be  
> used? It looks like, nobody ever updated the code for atomic, bond,  
> atompair descs to provide names!

> At this point, I'm leaning towards arbitrary names - the question is,  
> should the atom or bond in question be somehow indicated in the name?
>


I think it could be a nice idea if the names in question could starting with some of the three words
indicating Mblabla (Molecular), Ablabla(Atomic), Bblabla(Bond) descriptor.

> Out of curiosity, does anybody use these descriptors?
Bond descriptors are used by the IP calculations :(



-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Cdk-devel mailing list
Cdk-devel@...
https://lists.sourceforge.net/lists/listinfo/cdk-devel

[Rajarshi Guha-3]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


On Jul 16, 2008, at 11:37 PM, Rajarshi Guha wrote:
>>
>> Please go ahead. I'll monitor and comment where I feel I need to :)
>
> It's being done in the branch named rajasrshi-descnames

The refactoring in rajarshi-descnames is complete. A quick summary of  
what's been done:

1. IDescriptor now defines a method getDescriptorNames() that returns  
a String[] of individual descriptor names for that descriptor. The  
benefit is that it can be called without having to calculate the  
descriptor via calculate()

2. DescriptorValue now has a field and corresponding getter to hold  
an Exception object

3. The calculate() method in IAtomicDescriptor, IBondDescriptor,  
IAtomPairDescriptor, IMolecularDescriptor does not throw an exception  
anymore.

In general, descriptors will no longer throw exceptions. If  
exceptions do occur in a descriptor, they should be caught and the  
method should return a DescriptorValue object with the appropriate  
number of NaN's as the actual values and the exception object in  
question

4. All descriptors have been updated to follow the above policy

5. The descriptor test superclasses are updated to check that  
exceptions are not thrown by a descriptor and that the names obtained  
via IDescriptor and DescriptorValue are consistent with each other.

Comments welcome. If all is OK, I plan to merge with trunk next week

- -------------------------------------------------------------------
Rajarshi Guha  <rguha@...>
GPG Fingerprint: D070 5427 CC5B 7938 929C  DD13 66A1 922C 51E7 9E84
- -------------------------------------------------------------------
Psychology is merely producing habits out of rats.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (Darwin)

iEYEARECAAYFAkh/x2kACgkQZqGSLFHnnoSkLgCgvuZmw77u13fextGTb8TGp8bR
pDMAnRTgQ8z9xg6ERpMtDu/+Mk3T/Gcz
=Vjag
-----END PGP SIGNATURE-----

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Cdk-devel mailing list
Cdk-devel@...
https://lists.sourceforge.net/lists/listinfo/cdk-devel