Units #1389

Closed
wants to merge 33 commits into
from

Conversation

Projects
None yet
Contributor

melsophos commented Jun 27, 2012

Some preliminaries work on the unit systems. It's not over but Aaron advised to do a pull request to help the review (see https://groups.google.com/forum/?fromgroups#!topic/sympy/XbWxpKS6S94).
The current main problem comes from the operations, since Mul does not know that it should combine units and quantities together first (see the last test for an example).

Member

Krastanov commented Jun 27, 2012

The problem is that you have removed functionality that is probably already in use (the already defined constants in the old units module). Could you give a couple of example what the new module (or a future version of it) will be able to do that the current module can not. Maybe writing this in a new namespace and keeping the old module for some time would be a preferable solution.

By the way, try to follow the conventions for git messages (first line under 80 chars, empty line and then detailed description of what you are doing. Besides documentation in docstrings it is important to explain why a change is happening in the commit message).

And you need to rebase/merge this, as there are merge conflicts at the moment.

Member

Krastanov commented Jun 27, 2012

I checked quickly the code. It seems interesting and maybe more robust than the current module, however it would be nice to have examples of why this implementation should be preferred.

Member

Krastanov commented Jun 27, 2012

SymPy Bot Summary: ❗️ There were merge conflicts; could not test the branch.

@melsophos: Please rebase or merge your branch with master. See the report for a list of the merge conflicts.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYkPQdDA

Interpreter: /opt/pym32/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: aa989ce
branch hash: 07da182

Automatic review by SymPy Bot.

Contributor

melsophos commented Jun 27, 2012

Sorry for the bad commit messages (I have read the conventions, but too late for some), I should have explained in the first why I'm doing this. Maybe it will be better at the time of the true pull request to delete and create again the branch to do only one commit, since it will be really only one new feature, no? (I have separated because it was easier to write code in this way.)
Anyway it's the first time I'm doing such a wide patch, so if you see other remarks on the way to do it, just say.

I have also fixed the problem of merging, creating a new namespace as you advised.
Finally I have written some doc and an example in doc/src/modules/physics/unitsystems/index.txt. Say me if it's not sufficient (I have explained on the Google group discussion more in depth the reason of this implementation).

This pull request fails (merged e6c6b30 into aa989ce).

Member

Krastanov commented Jun 27, 2012

The pull request is already rather large. It would be better to have a number of commits in some natural progression. Otherwise it may be harder to review.

I saw your message on the mailing list, however I still do not understand why the new module is preferable (besides providing a more robust abstraction). In the old module one can work in astronomical units, for instance, in a rather clumsy way. my_length = a_number * au. It is clumsy because au itself is a a_number * m. Could you provide example that show how your module handles this? I see a number of examples, but you mention that the code for them is not written.

I think that Mul has already special code for dealing with units. To solve your problems you will be indeed forced to work with it.

Does x*m + y*m == (x+y)*m in your code? Are the hashes consistent?

@Krastanov Krastanov commented on the diff Jun 27, 2012

doc/src/modules/physics/unitsystems/index.txt
+ 3.33e5 earth_mass
+
+One has not to set the system since the units keep track of the system. It
+allows doing computations in several systems at the same time, without changing
+it explicitly.
+
+Compute the Venus revolution period:
+
+ >>> from sympy import pi, solve, Symbol
+ >>> solar_mass = astronomy['solar_mass']
+ >>> venus_a = Q('0.723332 ua')
+ >>> venus_T = solve(T**2/venus_a**3 - 4*pi**2 / G / solar_mass, T)[0]
+ >>> type(venus_T)
+ sympy.physics.unitsystems.units.Quantity
+ >>> venus_T
+ 0.61 year
@Krastanov

Krastanov Jun 27, 2012

Member

These doctests will raise errors for the moment as they are not implemented.

@melsophos

melsophos Jun 27, 2012

Contributor

You are right, but I did not know where to put these examples. If you think it's better I can move them on the wiki page.

@flacjacket

flacjacket Jun 27, 2012

Member

I think you can change the >>> to >> and it won't run the doctests, tho I'm not sure what that will do to the sphinx docs.

@melsophos

melsophos Jun 28, 2012

Contributor

Eventually I have used # doctest: +SKIP; it's not really beautiful but at least doctest succed.

@Krastanov Krastanov commented on the diff Jun 27, 2012

sympy/physics/unitsystems/dimensions.py
+
+"""
+Dimension for physical quantities.
+"""
+
+from __future__ import division
+from copy import copy
+
+from sympy.core.containers import Dict
+from sympy import Rational, Number, sympify
+
+# TODO: define the Dimension as a commutative Symbol, see paulialgebra module.
+# TODO: define a dimensionless fixed dimension, which is returned instead of 1
+# in operations
+
+class Dimension(Dict):
@Krastanov

Krastanov Jun 27, 2012

Member

Why Dict and not simply dict? You may need to check out the abc module in the standard library.

@melsophos

melsophos Jun 27, 2012

Contributor

The dict object is not cachable by sympy, as we spoke about in the topic https://groups.google.com/forum/?fromgroups#!topic/sympy/ga4AyRyE9Zw

@Krastanov

Krastanov Jun 27, 2012

Member

Actually, can't this be just a tuple? Length is just (1,0...) and time is (0,1,0...). I do not think that you need a whole new class for something that can be represented inside Unit.

@melsophos

melsophos Jun 27, 2012

Contributor

At the beginning I was using a tuple, but Aaron suggested the idea of a dict (#1940) to let the possibility to extend and to not fix a priori the number of different dimensions.

@Krastanov

Krastanov Jun 27, 2012

Member

At the beginning I was using a tuple, but Aaron suggested the idea of a dict (#1940) to let the possibility to extend and to not fix a priori the number of different dimensions.

I see. Is there a situation in which creating new dimensions is appropriate?

@melsophos

melsophos Jun 27, 2012

Contributor

I think it's the most useful when you have different system and that you don't care to keep track of the other. For example, if you have a system with mass/length/time and another with bytes, you don't want to mix them (you can, but you will have some useless extra information). Also the dict form allow to work with dimension names naturally, and avoid to have to remember the order.
Also if you have only the number of dimensions that you need, the methods that gives information on the dimensions and so on will return less, which is better. For example the matrix of dimensions will be only a 1x1 in case of bytes, 3x3 for mks...

@Krastanov Krastanov commented on the diff Jun 27, 2012

sympy/physics/unitsystems/dimensions.py
+# in operations
+
+class Dimension(Dict):
+ """
+ This class represent the dimension of a physical units.
+
+ Examples
+ ========
+
+ >>> from sympy.physics.unitsystems.mks import length, time
+ >>> velocity = length / time
+ >>> velocity
+ {length: 1, time: -1}
+ """
+
+ def __new__(cls, *args, **kwargs):
@Krastanov

Krastanov Jun 27, 2012

Member

Why not just __init__?

@melsophos

melsophos Jun 27, 2012

Contributor

I had some problems to make it working with the Dict class, and I found new easier to implement (I don't remember exactly but I think I saw a class elsewhere in the sympy code doing something similar).

@Krastanov Krastanov and 1 other commented on an outdated diff Jun 27, 2012

sympy/physics/unitsystems/units.py
+"""
+
+# TODO: main problem is that Mul(u1, u2) does not multiply the units
+# using __mul__
+
+from __future__ import division
+from copy import copy
+
+from sympy import (sympify, Number, Integer, Rational, Matrix, AtomicExpr,
+ Mul, Pow)
+
+# Is it a good idea to combine prefixes with between them, instead of just
+# keeping them to better print results when the user asks for it?
+# So do we want not to care about prefixes until the end?
+
+_UNIT_SYSTEM = None
@Krastanov

Krastanov Jun 27, 2012

Member

Such stateful code seems rather dangerous inside a library (sympy is not only for interactive use). And it is not really an object-oriented approach.

@melsophos

melsophos Jun 27, 2012

Contributor

I did not find other solution to change the system for all units and operations without specifying it at each time. But it was not intended to be use only in interactive mode.

@Krastanov

Krastanov Jun 27, 2012

Member

What about this:

us = UnitSystem_whatever_whatever()
us.some_method_that_does_whaetever()
x = us.some_attribute_that_I_will_use_often
@melsophos

melsophos Jun 28, 2012

Contributor

If I understand correctly, UnitSystem_whatever_whatever is a class having some methods to define global options, that the UnitSystem (and Unit, Quantitiy) class(es) will use?

@Krastanov

Krastanov Jun 28, 2012

Member

If I understand correctly, UnitSystem_whatever_whatever is a class having some methods to define global options, that the UnitSystem (and Unit, Quantitiy) class(es) will use?

There is nothing global and there should not be in my opinion. All the
stuff that you want to be global should be better encapsulated in an
object. All the work is done on this object, not on some global state
that can be corrupted at any time by some unrelated function.

At least this seems like a clearer solution to me.

@melsophos

melsophos Jun 28, 2012

Contributor

I understand your point, and in my opinion it would be the best way. But I don't see how to say at once to all unit what system should be used in priority if they don't come with "intrinsic" system (as it is the case when you get the unit from the system, which sould in principle be the preferred way).

@Krastanov

Krastanov Jun 28, 2012

Member

I understand your point, and in my opinion it would be the best way. But I don't see how to say at once to all unit what system should be used in priority if they don't come with "intrinsic" system (as it is the case when you get the unit from the system, which sould in principle be the preferred way).

I am not completely sure what you mean. Isn't "getting units from the
system" the only way to do it? If not, why not?

@melsophos

melsophos Jun 28, 2012

Contributor

If you want you can create unit without system, as is done at the beginning of the mks file. In principe one can wish to use these units without system, and only later chose one (I recognize it will be a rare case). Another example is the one I give in the documentation, where you first use the astronomical system and then the mks system: without the global set function you will have to change the system for each unit (at least for now I see no other way).

@Krastanov

Krastanov Jun 28, 2012

Member

What does "change the system" mean? Could you write example code?

@Krastanov Krastanov commented on the diff Jun 27, 2012

sympy/physics/unitsystems/units.py
+ 'k': Prefix('kilo', 'k', 3),
+ 'h': Prefix('hecto', 'h', 2),
+ 'da': Prefix('deca', 'da', 1),
+ 'd': Prefix('deci', 'd', -1),
+ 'c': Prefix('centi', 'c', -2),
+ 'm': Prefix('milli', 'm', -3),
+ 'µ': Prefix('micro', 'µ', -6),
+ 'n': Prefix('nano', 'n', -9),
+ 'p': Prefix('pico', 'p', -12),
+ 'f': Prefix('femto', 'f', -15),
+ 'a': Prefix('atto', 'a', -18),
+ 'z': Prefix('zepto', 'z', -21),
+ 'y': Prefix('yocto', 'y', -24)
+}
+
+class Unit(AtomicExpr):
@Krastanov

Krastanov Jun 27, 2012

Member

Unit does not seem to actually be atomic. It contains Dimension and a couple of other objects.

@melsophos

melsophos Jun 27, 2012

Contributor

Exact, but you will (almost ?) never write it as more atomic expressions: you can replace it by, for example, a dimension and a number, but we never do this. Si for me Unit was an atomic thing, even if inside it gathers several objects. But if you think Expr fits better I can change it.

@Krastanov Krastanov and 1 other commented on an outdated diff Jun 27, 2012

sympy/physics/unitsystems/units.py
+
+ #TODO: should m+m be interpreted as 1*m + 1*m?
+ #def __add__(self, other):
+ # if self != other:
+ # raise TypeError('Only identical units can be added.')
+ # else:
+ # return self
+
+ #def __sub__(self, other):
+ # if self != other:
+ # raise TypeError('Only identical units can be subtracted.')
+ # else:
+ # return self
+
+
+class UnitSystem():
@Krastanov

Krastanov Jun 27, 2012

Member

This should probably be a new style class.

@melsophos

melsophos Jun 28, 2012

Contributor

Exact, it will be fixed in next commit.

@Krastanov Krastanov commented on an outdated diff Jun 27, 2012

sympy/physics/unitsystems/units.py
+ __rtruediv__ = __rdiv__
+
+ def __pow__(self, other):
+
+ other = sympify(other)
+ if isinstance(other, Number):
+ return Quantity(self.factor**other, self.unit**other)
+ else:
+ return Pow(self, other)
+
+ def __eq__(self, other):
+ #TODO: interpret an Unit as a quantity with factor 1
+ return (isinstance(other, Quantity) and self.factor == other.factor
+ and self.unit == other.unit)
+
+class Constant(Unit):
@Krastanov

Krastanov Jun 27, 2012

Member

It may be useful to check the NumericConstant class.

Member

Krastanov commented Jun 27, 2012

I have made a number of comments about details that are not obvious to me (and maybe others). However my knowledge of the module is limited so a part of the comments may be caused simply by me misunderstanding the code.

This does look interesting. I wonder how much of it can be glued onto the old module.

Member

Krastanov commented Jun 27, 2012

SymPy Bot Summary: 🔴 There were test failures.

@melsophos: Please fix the test failures.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY1oMeDA

Interpreter: /opt/pym32/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: aa989ce
branch hash: e6c6b30

Automatic review by SymPy Bot.

Member

Krastanov commented Jun 27, 2012

SymPy Bot Summary: 🔴 There were test failures.

@melsophos: Please fix the test failures.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYqOwdDA

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: aa989ce
branch hash: e6c6b30

Automatic review by SymPy Bot.

Member

Krastanov commented Jun 27, 2012

SymPy Bot Summary: 🔴 There were test failures.

@melsophos: Please fix the test failures.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYtOQdDA

Interpreter: /usr/bin/python3 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: aa989ce
branch hash: e6c6b30

Automatic review by SymPy Bot.

@mrocklin mrocklin and 1 other commented on an outdated diff Jun 27, 2012

sympy/physics/unitsystems/mks.py
+from sympy.physics.unitsystems.dimensions import Dimension
+from sympy.physics.unitsystems.units import Unit, UnitSystem, PREFIXES
+
+# base dimensions
+length = Dimension(name='length', symbol='L', length=1)
+mass = Dimension(name='mass', symbol='M', mass=1)
+time = Dimension(name='time', symbol='T', time=1)
+
+# derived dimensions
+velocity = Dimension(name='velocity', length=1, time=-1)
+acceleration = Dimension(name='acceleration', length=1, time=-2)
+momentum = Dimension(name='momentum', mass=1, length=1, time=-1)
+force = Dimension(name='force', mass=1, length=1, time=-2)
+energy = Dimension(name='energy', mass=1, length=2, time=-2)
+power = Dimension(name='power', mass=1, length=2, time=-3)
+pressure = Dimension(name='pressure', mass=1, length=-1, time=-2)
@mrocklin

mrocklin Jun 27, 2012

Member

The base and derived dimensions look as if they are more general than MKS. Should they live somewhere else?

@melsophos

melsophos Jun 27, 2012

Contributor

I'm not exactly sure because these dimensions have a sense only within the MKS system. But you can derive system from other one, for example getting MKSA from MKS by just adding the current and the ampere as base.
Also if for example you want to deal with data quantities, you will need only this dimension and bytes/octets, so other dimensions are useless and should not appeared.

Contributor

melsophos commented Jun 28, 2012

I saw your message on the mailing list, however I still do not understand why the new module is preferable (besides providing a more robust abstraction). In the old module one can work in astronomical units, for instance, in a rather clumsy way. my_length = a_number * au. It is clumsy because au itself is a a_number * m. Could you provide example that show how your module handles this? I see a number of examples, but you mention that the code for them is not written.

I think that just the robustness and the simplifications that occured justify the new module, even if it's sure that the old one is sufficient for basic things.

Contributor

melsophos commented Jun 28, 2012

I think that Mul has already special code for dealing with units. To solve your problems you will be indeed forced to work with it.

Does x_m + y_m == (x+y)*m in your code? Are the hashes consistent?

If x and y are symbols, and m an unit, I have:

>>> x*m + y*m == (x+y)*m
False
>>> (x*m + y*m).factor() == (x+y)*m
True

Concerning the hashes I don't know.

johanhake and others added some commits Jul 10, 2012

@johanhake johanhake Small format changes:
  * Changed all "an unit" to "a unit"
  * Fixed a typo
  * Use same string deliminator (""") for docstrings everywhere
a8ecf17
@johanhake johanhake Added simplify_units function, which "fixes" problem with Mul (and Po…
…w) of

a mix of units and other sympy objects, which prevented simplifications of
units in complicated expressions.
d9cf59c
@melsophos melsophos Merge pull request #1 from johan-hake/units
Small format changes and add function to simplify units.
bc9fd57

This pull request fails (merged bc9fd57 into aa989ce).

Member

Krastanov commented Jul 13, 2012

SymPy Bot Summary: 🔴 There were test failures.

@melsophos: Please fix the test failures.

Test command: setup.py test
master hash: 61951c2
branch hash: bc9fd57

Interpreter 1: 🔴 There were test failures.

Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY0NYfDA

Interpreter 2: 🔴 There were test failures.

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYv4gfDA

Interpreter 3: 🔴 There were test failures.

Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY1aQgDA

Build HTML Docs: 🔴 There were test failures.

Docs build command: make html-errors
Sphinx version: 1.1.3

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY7pwgDA

Automatic review by SymPy Bot.

Member

jrioux commented Jul 16, 2012

SymPy Bot Summary: 🔴 There were test failures.

@melsophos: Please fix the test failures.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYp7QgDA

Interpreter: /usr/bin/python (2.7.0-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: fb35324
branch hash: bc9fd57

Automatic review by SymPy Bot.

Owner

certik commented Aug 11, 2012

@melsophos, are you interested in finishing this? There are currently some test failures.

Contributor

melsophos commented Aug 11, 2012

I clearly want to finish this, but for now I have to work on my PhD so I had to put this on the side. I made some corrections recently, so I hope it will fix some failures when I will push them. Sorry to do this so slowly.

Owner

certik commented Aug 11, 2012

No problem. I also have to work on my PhD, so I understand exactly.

Member

Krastanov commented Aug 25, 2012

SymPy Bot Summary: 🔴 Failed after merging melsophos/units (bc9fd57) into master (a184841).
@melsophos: Please fix the test failures.
🔴 Python 2.5.6-final-0: fail
🔴 Python 2.7.3-candidate-2: fail
🔴 Python 3.2.3-candidate-2: fail
🔴Sphinx 1.1.3: fail

Member

jrioux commented Sep 13, 2012

SymPy Bot Summary: 🔴 Failed after merging melsophos/units (bc9fd57) into master (c49dca1).
@melsophos: Please fix the test failures.
🔴 Python 2.7.2-final-0: fail
🔴 Python 3.2.1-final-0: fail
✳️Sphinx 1.1.3: pass
Docs build command: make clean && make html-errors && make clean && make latex && cd _build/latex && xelatex -interaction=nonstopmode sympy-*.tex

@melsophos melsophos Add Constant class, fix unit simplication and errors in tests.
For now the Constant class just inherits from the Unit class
without any extra structure.
Automatic conversions have been removed to avoid magic in
computations (which would have certainly make problems in future
improvements for constants). This has lead to a number of changes.
Errors from bin/test (e.g. in the module of str printing) have
been fixed.
cd12f05
Member

jrioux commented Oct 15, 2012

SymPy Bot Summary: 🔴 Failed after merging melsophos/units (cd12f05) into master (250d764).
@melsophos: Please fix the test failures.
✳️ Python 2.7.2-final-0: pass
🔴 Python 3.2.1-final-0: fail
✳️Sphinx 1.1.3: pass
Docs build command: make clean && make html-errors && make clean && make latex && cd _build/latex && xelatex sympy-*.tex

Member

jrioux commented Oct 22, 2012

SymPy Bot Summary: ❗️ There were merge conflicts (could not merge melsophos/units (cd12f05) into master (57e94e4)); could not test the branch.
@melsophos: Please rebase or merge your branch with master. See the report for a list of the merge conflicts.

nim65s and others added some commits Nov 5, 2012

Member

jrioux commented Nov 8, 2012

SymPy Bot Summary: ❗️ There were merge conflicts (could not merge melsophos/units (67cb3a4) into master (4055c4b)); could not test the branch.
@melsophos: Please rebase or merge your branch with master. See the report for a list of the merge conflicts.

melsophos added some commits Dec 28, 2012

@melsophos melsophos Add mechanism to extend systems. 6e212ff
@melsophos melsophos Merge branch 'master' of git://github.com/sympy/sympy into units
Conflicts:
	sympy/core/tests/test_args.py
	sympy/printing/tests/test_str.py
05352dc
@melsophos melsophos Define MKSA system by extending MKS.
Rename MASK to MKSA. Fix also extend method of UnitSystem.
b239662
Owner

asmeurer commented Jan 11, 2013

Can this reach a state where it can just be merged as a new, possibly experimental module? I think it would get a lot more attention that way.

Owner

asmeurer commented Jan 11, 2013

SymPy Bot Summary: ✳️ Passed after merging melsophos/units (b239662) into master (5cb0ea9).
✳️ Python 2.7.3-final-0: pass

Owner

asmeurer commented Jan 12, 2013

SymPy Bot Summary: 🔴 Failed after merging melsophos/units (b239662) into master (c8729f6).
@melsophos: Please fix the test failures.
✳️ Python 2.5.0-final-0: pass
✳️ Python 2.6.6-final-0: pass
✳️ Python 2.7.2-final-0: pass
✳️ Python 2.6.8-final-0: pass
✳️ Python 2.7.3-final-0: pass
✳️ PyPy 2.0.0-beta-1; 2.7.3-final-42: pass
🔴 Python 3.2.2-final-0: fail
🔴 Python 3.3.0-final-0: fail
🔴 Python 3.2.3-final-0: fail
🔴 Python 3.3.0-final-0: fail
🔴 Python 3.3.0-final-0: fail
✳️ Sphinx 1.1.3: pass

vsilv commented Jan 19, 2013

I am very eager to have this implemented, can I somehow contribute (otherwise I'll find a solution on my own which I think is not the best way)? To what extend this is finished and what features are usable at the moment?

Owner

asmeurer commented Jan 19, 2013

If @melsophos isn't too busy, you could send pull requests to this branch, and he will merge them here. If he is too busy, you should open a new pr here based off this branch.

Contributor

melsophos commented Jan 26, 2013

Hi vsilv. I think that the core is almost done: I have still to implement the conversion between different systems, but it should not be too long (when I will begin it; I hope in the coming weeks).
Anyway you can take a look and send me pull request if you find something to improve, I always have time to review them.

Owner

asmeurer commented Feb 25, 2013

SymPy Bot Summary: 🔴 Failed after merging melsophos/units (b239662) into master (3523e7e).
@melsophos: Please fix the test failures.
✳️ Python 2.5.6-final-0: pass
✳️ Python 2.6.8-final-0: pass
✳️ Python 2.7.3-final-0: pass
🔴 Python 3.2.3-final-0: fail
✳️ Sphinx 1.1.3: pass

vsilv commented Feb 25, 2013

Thank you for the information, that will be very usefull for me. I will
give it a shoot on weekend!
with best regards, vsilv

2013/2/25 Aaron Meurer notifications@github.com

SymPy Bot https://github.com/sympy/sympy-bot Summary: [image:
🔴] Failed after merging melsophos/units (b239662b239662)
into master (3523e7ehttps://github.com/sympy/sympy/commit/3523e7e0998f50dbcd60914612e9764b32411590
).
@melsophos https://github.com/melsophos: Please fix the test failures.
[image: ✳️]Python 2.5.6-final-0: passhttp://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYleBqDA
[image: ✳️]Python 2.6.8-final-0: passhttp://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYluBqDA
[image: ✳️]Python 2.7.3-final-0: passhttp://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYq_9qDA
[image: 🔴]Python 3.2.3-final-0: failhttp://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYtKZrDA
[image: ✳️]Sphinx 1.1.3: passhttp://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYl-BqDA


Reply to this email directly or view it on GitHubhttps://github.com/sympy/sympy/pull/1389#issuecomment-14054048.

Member

jrioux commented Apr 9, 2013

SymPy Bot Summary: 🔴 Failed after merging melsophos/units (4352783) into master (e639afb).
@melsophos: Please fix the test failures.
🔴 PyPy 2.0.0-beta-1; 2.7.3-final-42: fail
🔴 Python 2.7.2-final-0: fail
🔴 Python 3.2.1-final-0: fail
🔴 Sphinx 1.1.3: fail
Docs build command: "make clean && make html-errors && make latex && cd _build/latex && xelatex sympy-*.tex"

Contributor

Upabjojr commented Apr 12, 2013

Hi, I opened a pull request to use Context Managers with UnitSystem objects, that will allow to use the with statement instead of set_system( ... )

I also made a small review to the tests, now they pass.

By the way, the Astronomical Unit isn't 1.49597870700e11 meters? In your code I see the exponentiation up to e14, instead of e11. Is it a mistake or are you using millimeters?

If you agree, I can add a list of constants and collect data about various unit systems from the internet.

Contributor

Upabjojr commented Apr 12, 2013

By the way, I don't agree on some logic of the code.

The Quantity( ... ) object should always be a Quantity, not a Number. There are some constants which are pure numbers in some unit systems, while having units on other ones. I mean, some unit systems change even dimensions of quantities (not just their units).

Besides, I think that Quantity's and Constant's should be Unit System-independent, and have a unit-system-dependent evalf( ). Physical constants and quantities are universal concepts, the symbols representing them must be always the same.

Besides, I suggest to further introduce an object "MeasurableSymbol", representing symbolic values which can undergo a change in representation when switching through unit measurement systems.

For example, the electric field E in SI becomes E/c in Gauss units.

In order to represent a Quantity, I would introduce a MostGeneralUnitSystem, maybe at first by extending the International System. The internal representation of a Quantity shall always be its magnitude in the MostGeneralUnitSystem, while evalf( ... ), n( ... ), str( ... ) and _pprint( ... ) methods shall return the value in the currently default unit system.

Owner

asmeurer commented Apr 12, 2013

(for reference: melsophos#3)

Owner

asmeurer commented Apr 13, 2013

SymPy Bot Summary: Passed after merging melsophos/units (4352783) into master (00a0a1e).
Sphinx 1.2b1: pass
Doc Coverage: decreased
36.48% of functions have doctests (compared to 36.58% in master)
40.85% of functions are imported into Sphinx (compared to 40.97% in master)

Contributor

Upabjojr commented Apr 13, 2013

Setting a unit system looks like using an assumption in the new assumptions engine. Aaron suggested to add an assumption to their engine specifying the default unit system. That can be done in the future, I think it's better to wait until the new assumptions engine is fully integrated into SymPy. Maybe open a ticket/issue for that?

I think that this pull request should be completed as soon as possible, because much of the Physics module may potentially depend on this. For example, Physics textbooks about Quantum Field Theory usually adopt natural units or a similar strategy in order to simplify the equations (otherwise they would be a nightmare to read), but someone may be willing to verify those equations with experimental data, thus he would need to evaluate them in MKS or something else.

As another exmple, consider Relativity. There, time and length become the same dimension. I think that the unit system engine should be able to handle that, before starting any implementation of Relativity here. One has to both view a hypothetical future Time( ) object, both by its value in MKS, in meter-equivalents (through speed_of_light conversion) and in Natural Units, etc... But the instance of Time( ) has always to be the same (except for Lorentz transformations, which is another kind of problem).

Long story short, it would be advisable to have the unitsystem working before implementing physical laws and quantities.

If you agree, I can go on with your work.

Contributor

Upabjojr commented Apr 14, 2013

I had another idea today. Concerning symbolic physics (e.g. electric field symbol: E), in order to deal with different unit system representations, we could just overload a method to give dictionary-like access to the symbol, which means something like this:

E = ElectricField('E', value= [3*Volt/meter, 0, 0])

E # unitsystem-independent usage
E[SI] # representation in mks
E[gauss] # representation in gauss units

with SI: E
E[SI]

with gauss: E
E[gauss]/speed_of_light

Summary: we should have two forms of conversions, a numerical one acting on values of quantities, and a symbolic one, implementing a conversion of physical symbols.

Let me know your opinion.

Contributor

melsophos commented Apr 28, 2013

(Edit : my first comment was badly formatted for a reason I ignore, so I deleted it and post it again.)
(I'm answering in chronological order, so there can be some overlap of my answer to what you say later.)

Hi, I opened a pull request to use Context Managers with UnitSystem objects, that will allow to use the with statement instead of set_system( ... )

I also made a small review to the tests, now they pass.

By the way, the Astronomical Unit isn't 1.49597870700e11 meters? In your code I see the exponentiation up to e14, instead of e11. Is it a mistake or are you using millimeters?

If you agree, I can add a list of constants and collect data about various unit systems from the internet.

Thank a lot for your contribution. I will try to review the code soon, but this week I have exams to prepare for my teachings so I'm not sure to be able to do so. You are right concerning the ua, I made a mistake.

I did not want to define all the different unit systems yet because the core is not finished and the way we define unit systems could change. So the few systems, units and constants already defined are there just in order to test.
But then when it will be finished it will be very nice to collect all these data.

By the way, I don't agree on some logic of the code.

The Quantity( ... ) object should always be a Quantity, not a Number. There are some constants which are pure numbers in some unit systems, while having units on other ones. I mean, some unit systems change even dimensions of quantities (not just their units).

Concerning the quantity I was hesitating, so we can discuss of this. For example my current code do not permit the use of "true" numerical constant like the fine structure constant. But it can always be implemented as a constant number like pi. But on the other hand a constant either is a numerical constant in all unit systems (so like the fine structure constant), or either it is always a "dimensionfull constant": in fact, if a physical constant appears to be a dimensionless constant in some system, like c=1 in special relativity, it's only because we choose to use this constant as a unit, i.e. the velocity (which is the dimension of c) is now a base dimension (instead of the time for example), c is a base unit (by itself) ; so the correct interpretation is that when we fix a constant to 1 (e.g.) then it becomes a base unit in the same way as the m (meter) symbol exists by itself which means that m = 1 in this language. But we should distinguish between the numerical value (which base units have not - really it's 1) and dimension value (which is always here). Another way to see this: unit systems are a group in the mathematical sense, whose generators are the base units. Changing a unit system to an equivalent one is just doing a change of basis. So if we start with a 2-dimensional system (m,s) with base dimension (length, time), and decide to change for the system where c=1, what we really do is defining a new system with base units (m, c) and dimension (length, velocity); else we would loose informations (this we can do as human because we now dimensional analysis and can get back the factors of c and others, but computers are not able to do so; because it's physics, not mathematics). And this does change the dimension of all physical quantities when expressed in terms of the new basis (but the "absolute" dimension does not change, because we have to start somewhere to define all the dimensions, and so there is an "absolute" order, but when we have begun, this is invisible). But this is not a problem and already implemented with my use of vectors to represent dimension.

Besides, I think that Quantity's and Constant's should be Unit System-independent, and have a unit-system-dependent evalf( ). Physical constants and quantities are universal concepts, the symbols representing them must be always the same.

In fact my last commits are not we satisfying concerning this point, because I tried to implement a user-friendly displaying which always use the best xay to display, but this block the conversion to other (non-base) units. In principle I wanted to define units and quantities to be globally independent of systems, but it's quite difficult, because in principle there is no sense in taking a quantity outside a system: how do you define its value? with respect to which origin? I had the same problem with the definition of dimensions, and basically the answer is: take a conventional origin (like length, time, etc.), define everything (other dimensions, units, quantities, constants) with respect to this, introduce a system, put all into it, and then you can forget about the "order" you define previously. So I don't totally agree that quantities and constants are universal concepts, you always have to define them with respect to something. For me what you want to define as "universal" symbol are more symbols, for which we can use the sympy Symbol, and later replace it with a quantity, or something else. So maybe it would be useful to have an intermediate level between the sympy Symbol and the unitsystem Quantity, but the later should really be dependent of systems.

Besides, I suggest to further introduce an object "MeasurableSymbol", representing symbolic values which can undergo a change in representation when switching through unit measurement systems.

For example, the electric field E in SI becomes E/c in Gauss units.

This is already what Quantity is doing, so I think we are not using the same words for the concepts, but the idea is the same here. The conversion is the part I have not finished to prepare.

In order to represent a Quantity, I would introduce a MostGeneralUnitSystem, maybe at first by extending the International System. The internal representation of a Quantity shall always be its magnitude in the MostGeneralUnitSystem, while evalf( ... ), n( ... ), str( ... ) and _pprint( ... ) methods shall return the value in the currently default unit system.

In some way it is what I'm doing by defining my dimensions with some conventions (see for example in MKS) and I use them after to define everything. With this method the problem is that nothing enforce that the length define somewhere is the same as the length define elsewhere, since there is no global general system. But as I said this is not possible because in order to first define a system, we need to define dimensions and so on! It's chicken or the egg. And a most general unit system is not necessarily desirable because the user should be able to define systems in a totally different context than physics, for example with computer units, calendars, etc.

Setting a unit system looks like using an assumption in the new assumptions engine. Aaron suggested to add an assumption to their engine specifying the default unit system. That can be done in the future, I think it's better to wait until the new assumptions engine is fully integrated into SymPy. Maybe open a ticket/issue for that?

I did not know about this. It could be interesting, but it's not the main problem for now, I still have to finish to define the conversion between units and systems.

I think that this pull request should be completed as soon as possible, because much of the Physics module may potentially depend on this. For example, Physics textbooks about Quantum Field Theory usually adopt natural units or a similar strategy in order to simplify the equations (otherwise they would be a nightmare to read), but someone may be willing to verify those equations with experimental data, thus he would need to evaluate them in MKS or something else.

I agree with you, it is what I'm working on currently.

As another exmple, consider Relativity. There, time and length become the same dimension. I think that the unit system engine should be able to handle that, before starting any implementation of Relativity here. One has to both view a hypothetical future Time( ) object, both by its value in MKS, in meter-equivalents (through speed_of_light conversion) and in Natural Units, etc... But the instance of Time( ) has always to be the same (except for Lorentz transformations, which is another kind of problem).

As I explained above, this point of view is totally wrong: length and time appears to us to be the same dimension, but really time is length velocity^-1.

Long story short, it would be advisable to have the unitsystem working before implementing physical laws and quantities.

If you agree, I can go on with your work.

My teachings are over by two weeks, so I should have time to work again on this. So let me few weeks to finish the things I was working on, and then I will be glad to get your help on the following.

I had another idea today. Concerning symbolic physics (e.g. electric field symbol: E), in order to deal with different unit system representations, we could just overload a method to give dictionary-like access to the symbol, which means something like this:

E = ElectricField('E', value= [3*Volt/meter, 0, 0])

E # unitsystem-independent usage E[SI] # representation in mks
E[gauss] # representation in gauss units

with SI: E E[SI]

with gauss: E E[gauss]/speed_of_light

Summary: we should have two forms of conversions, a numerical one acting on values of quantities, and a symbolic one, implementing a conversion of physical symbols.

This proposition is interesting for some part, but some points disagree with the view I presented before. So we should see what can be done.

But anyway I think it's good to keep in mind the future features we want, but that we should not work on them now since the core is not finished.

Contributor

Upabjojr commented Apr 29, 2013

I think I've uploaded my pull request again. Sorry for deleting it, I'm currently not that practical with git.

Member

jrioux commented Jul 7, 2013

SymPy Bot Summary: 🔴 Failed after merging melsophos/units (1ae0acd) into master (db939b6).
@melsophos: Please fix the test failures.
PyPy 2.0.0-beta-1; 2.7.3-final-42: pass
Python 2.7.2-final-0: pass
🔴 Python 3.2.1-final-0: fail
Sphinx 1.1.3: pass
Doc Coverage: decreased
36.179% of functions have doctests (compared to 36.276% in master)
40.411% of functions are imported into Sphinx (compared to 40.519% in master)

melsophos closed this Nov 3, 2013

melsophos deleted the melsophos:units branch Nov 3, 2013

Contributor

melsophos commented Nov 6, 2013

Hi all,
I had some problem with my git repo so I have deleted the branch, which had the effect to close the pull request. But the development continues, with more doc, tests and with simpler code; I will soon do a new pull request for discussion.
I apologize for the inconvenience.

Owner

certik commented Nov 6, 2013

No worries. Ping us once you want us to review it.

melsophos referenced this pull request Nov 24, 2013

Merged

Units #2628

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment