[ase-users] VASP module in ASE

[Jakob Schiotz]

On Monday 01 September 2008, Jens Jørgen Mortensen wrote:

Just a few comments to Jens Jørgen's comments

>  class Vasp:
> -    def __init__(self, restart=False, **kwargs):
> +    def __init__(self, restart=None, **kwargs):
>
> ??? use None for default values.
>

I think for default values one should either use None to mark it as a default, 
or (if possible and not too messy) the actual value that will be used as 
default.  Then it is more clear what the default is.  

>          if self.input_parameters['xc'] not in ['91','lda','pbe']:
> -            raise Exception, "%s not supported for xc! use one of
> ['91',lda','pbe']" % kwargs['xc']
> -        self.positions=None
> -        self.nbands=self.incar_parameters['nbands']
> -        self.atoms=None
> +            raise Exception(
> +                "%s not supported for xc! use one of ['91',lda','pbe']"
> %
> +                kwargs['xc'])
>
> ??? use the syntax Exception('text')

Is there a reason for this?  I have always been using the other syntax :-)

By the way, shouldn't we avoid exceptions of type Exception, this is the 
baseclass for all exceptions and is probably best left alone and used only 
for that purpose.  Instead one can use RuntimeError unless something else 
like ValueError or TypeError seems more appropriate.


> @@ -117,9 +119,7 @@
>              elif self.incar_parameters.has_key(key):
>                  self.incar_parameters[key]=kwargs[key]
>              else:
> -                print 'Parameter '+key+' not defined'
> -                return
> -
> +                raise TypeError('Parameter not defined: ' + key)
>
> ??? A wrong key should raise an exception.

But I think this should technically be a ValueError.  The variable key does 
not have the wrong type, just the wrong value.


Just my 2 cents.

/Jakob


-- 
Jakob Schiotz, Ph.D., Associate professor (lektor)
CINF, Department of Physics
Technical University of Denmark
DK-2800 Kongens Lyngby, Denmark
http://www.cinf.dtu.dk/~schiotz/