temperature.val does not return temperature but only its unit

[ppxasjsm]

I seem to notice something strange the way the temperature parameter is passed to python.
I use the default temperature value and don't overwrite with anything in my config file, i.e. this should be read:

temperature = Parameter("temperature", 25 * celsius, """Simulation temperature""")
pressure = Parameter("pressure", 1 * atm, """Simulation pressure""")

The output I get for the two variables is:

In [1]: temperature.val
Out[1]: SireUnits::Celsius

In [2]: pressure.val
Out[2]: 1 atm

Which is quite useless information for the temperature. Not sure where the problem might be.
The temperature does get correctly passed to c++ code though.

[jmichel80]

Sounds like a bug with the string representation of SireUnits::Celsius.
Do you get the same behaviour if you replace

temperature = Parameter("temperature", 25 * celsius, """Simulation
temperature""")

by

temperature = Parameter("temperature", 298 * kelvin, """Simulation
temperature""")

---

Dr. Julien Michel,
Royal Society University Research Fellow
Room 263, School of Chemistry
Joseph Black Building, University of Edinburgh
David Brewster Road

Edinburgh

EH9 3FJ
United Kingdom
phone: +44 (0)131 650 4797

http://www.julienmichel.net/

On Wed, Aug 12, 2015 at 3:05 PM, ppxasjsm notifications@github.com wrote:

I seem to notice something strange the way the temperature parameter is
passed to python.
I use the default temperature value and don't overwrite with anything in
my config file, i.e. this should be read:

temperature = Parameter("temperature", 25 * celsius, """Simulation temperature""")
pressure = Parameter("pressure", 1 * atm, """Simulation pressure""")

The output I get for the two variables is:

In [1]: temperature.val
Out[1]: SireUnits::Celsius

In [2]: pressure.val
Out[2]: 1 atm

Which is quite useless information for the temperature. Not sure where the
problem might be.
The temperature does get correctly passed to c++ code though.

—
Reply to this email directly or view it on GitHub
#50.

The University of Edinburgh is a charitable body, registered in
Scotland, with registration number SC005336.

[ppxasjsm]

Now I get this:

In [1]: temperature.val
Out[1]: 300 t # I used 300 * kelvin as input

[jmichel80]

will flag this to Chris as this definitely looks like a problem in SireUnits that ought to be fixed for 15.1

[chryswoods]

Yes, this is a bug. Temperature is the most painful unit to implement as it does not behave in the same way as other physical units. I have now downloaded Sire from github (devel branch) and will try to fix this problem over the coming week. It will be a test of whether or not I can successfully develop against the github repository (tear in my eye as I say goodbye to google code...)

p.s. The github repository looks great - you've done a brilliant job copying everything over.

[jmichel80]

Watch out for trailing white spaces ! Sure way to annoy Antonia !

---

Dr. Julien Michel,
Royal Society University Research Fellow
Room 263, School of Chemistry
Joseph Black Building, University of Edinburgh
David Brewster Road

Edinburgh

EH9 3FJ
United Kingdom
phone: +44 (0)131 650 4797

http://www.julienmichel.net/

On Wed, Sep 2, 2015 at 10:35 AM, Christopher Woods <notifications@github.com

wrote:

Yes, this is a bug. Temperature is the most painful unit to implement as
it does not behave in the same way as other physical units. I have now
downloaded Sire from github (devel branch) and will try to fix this problem
over the coming week. It will be a test of whether or not I can
successfully develop against the github repository (tear in my eye as I say
goodbye to google code...)

p.s. The github repository looks great - you've done a brilliant job
copying everything over.

—
Reply to this email directly or view it on GitHub
#50 (comment).

The University of Edinburgh is a charitable body, registered in
Scotland, with registration number SC005336.

[chryswoods]

Annoys me too ;-)

My pet peeve though is "incorrect" indenting, e.g. not using 4 spaces to indent, and not putting the braces in the right place. I am too picky.

I've now compiled corelib on my mac. I've had to fix a small compile bug that I will commit back to the repository. I will then work on compiling the wrappers, which I know will be more painful as I am using Yosemite. I have time this month to really look at Sire, so will put everything in place so that we can look to release either at the end of this month or after the workshop in Juelich.

[ppxasjsm]

White spaces are mainly annoying when you want to track the changes someone made to a file and you don't want to see all the 100 lines where white spaces were added.

Indenting is also annoying....also, it would be great if we can have a bit more detailed chat about Sire in Jülich.

[chryswoods]

Closed as fixed in the latest push to merge_2014_4

[ppxasjsm]

I just noticed that in the devel branch, the Kelvin temperature unit, does not seem work:

>>> from Sire.Units import *
>>> t = 15 * kelvin
>>> t
15 t

Also:

>>> t = 15*celsius
>>> t.val
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'Celsius' object has no attribute 'val'

[chryswoods]

The fix matched the API for temperature to that for GeneralUnit. This means that there is no .val member variable. Instead you have to call .value() to get the value in internal units, or call .to() to convert the value to the desired units, e.g.

from Sire.Units import *
t = 15 * celsius
t.value()
288.15
t.to(fahrenheit)
58.9999999994

The GeneralUnit class will try to write out the unit string with the value (e.g. C for Celsius, A for angstrom etc.). However, I have not added in all of the unit strings for all of the units, e.g. base temperature (kelvin) is written using 't' for temperature, while base mass (units of atomic mass constant) is written using 'm' for mass.

If you want to add unit strings, then edit getUnitString in corelib/src/libs/SireUnits/convert.cpp. This initialises a dictionary that maps all of the 'common' units used in Sire to unit strings.

[chryswoods]

Closing as no progress