santoshphilip / eppy

scripting language for E+, Energyplus
MIT License
151 stars 66 forks source link

write functions that work work when numpy is not installed #30

Closed santoshphilip closed 8 years ago

santoshphilip commented 9 years ago

There are geometry functions in ./eppy/geometry that use numpy. It can be difficult to install numpy at times (for instance if you want to run epp in rhino http://www.rhino3d.com)

eayoungs commented 9 years ago

I've taken the first stab at re-writing the np.cross() function in Python, including a new test function.

They are located in the py_numeric.py and test_py_numeric.py files, respectively, within the geometry/ and tests/geometry_tests folders on the new 'i30_pythongeometry' branch.

eayoungs commented 9 years ago

I've updated the style of my code to conform with PEP-8. surface.py itself has some non-conformities but I updated the module to conform up to the point that I need to modify (the import statements). I plan to ignore the rest of the module for now. Please confirm this is the recommended approach

I'd like to modify it's test prior to modifying the surface.py module. My first inclination is to set up a virtual environment where numpy is not installed so that the try statement will fail but I'm not sure how to make this an explicit part of the test.

Please advise.

eayoungs commented 9 years ago

Can I assume that all vectors passed to the 'cross' function (cross product) are going to be 3-dimentional? This would greatly simplify the function. Also, can I assume this for any (or all) of the other numpy functions used?

The original author of the Geometry library, Tuan Trang replied in an offline email that this is a valid assumption, so I have continued based on this information.

eayoungs commented 9 years ago

I added a dot product function & test today.

santoshphilip commented 9 years ago

Eric, I am in India. I have the code with me. My response time will be slow.

S.

Sent from my iPad

On Dec 9, 2014, at 2:08 PM, Eric Youngson notifications@github.com wrote:

I added a dot product function & test today.

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

eayoungs commented 9 years ago

No worries Santosh. I need to start on documentation and expanding test cases.

On 12/11/14 2:04 PM, santoshphilip wrote:

Eric, I am in India. I have the code with me. My response time will be slow.

S.

Sent from my iPad

On Dec 9, 2014, at 2:08 PM, Eric Youngson notifications@github.com wrote:

I added a dot product function & test today.

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

— Reply to this email directly or view it on GitHub https://github.com/santoshphilip/eppy/issues/30#issuecomment-66697519.

eayoungs commented 9 years ago

I've revised the vector cross product function and tests to use exception handling and assert that exceptions are raised in the tests. I just got the tests to pass and will be moving on to use the same pattern to revise the rest of the functions I've written so far (dot product, determinant) before moving on to other functions.

Please review and advise as to weather the current approach is satisfactory before I move on

Thx!

Eric

santoshphilip commented 9 years ago

Awesome ! I'll review and respond.

Santosh.

Sent from my iPad

On May 9, 2015, at 8:09 PM, Eric Youngson notifications@github.com wrote:

Santosh

I've revised the vector cross product function and tests to use exception handling and assert that exceptions are raised in the tests. I just got the tests to pass and will be moving on to use the same pattern to revise the rest of the functions I've written so far (dot product, determinant) before moving on to other functions.

Please review and advise as to weather the current approach is satisfactory before I move on

Thx!

Eric

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

santoshphilip commented 9 years ago

Eric, You are on the right track. Please continue.

I have some thoughts on coding style (PEP 8) But please do not do anything about code style right now. There automated tools to review the style and update. Once you are finished we will apply those tools.

Looks like you are getting close to wrapping this up.

Santosh

eayoungs commented 9 years ago

Great! Thanks for the feedback. I feel like I'm starting to get the hang of it.

santoshphilip commented 9 years ago

Eric,    You have been programming rather defensively so far, trying to get things right.You are on the right track.You can now code a little more aggressively now. In fact that is the thinking behind unit testing.- you write the test first- the test will fail because you have no code- write the code- the test may succeed or fail- write more tests and continue- stop when you think it is good enough - not when it is perfect. Basically you making the computer catch your mistakes. Later you clean the style, refactor, document better etc. At the end you transition to python3.

Santosh

eayoungs commented 9 years ago

Sounds good. I will keep that in mind and try to follow your advice. Sounds like the idea is to move more quickly and allow for more experimentation. I'm excited to start a new phase in my coding!

eayoungs commented 9 years ago

Santosh,

I started writing the replacement to numpy.array(), which encompasses a lot of functionality. I was thinking I might need to write a class since there are many methods associated with it.

Then I took a look at where it's actually used within eppy. I only find references to it in the int2lines module. Furthermore, I don't see it imported or even referenced anywhere else in the project. If this is the case, I wonder if there is a good justification for re-implementing it in pure Python. I will move on to the last 2 numpy functions used; square root, and arc-cosine.

Cheers

Eric

santoshphilip commented 9 years ago

don't write anything you don't have to Also see if you can find the other function in the math library do import math and check

vinc85 commented 9 years ago

How can I import eppy in a java web application using Jython, with dependencies of numpy? thanks

santoshphilip commented 9 years ago

Eric, To get a background in this, take a look at issue #48 and my comment in https://groups.yahoo.com/neo/groups/eppy/conversations/messages/3

Vinc85 can use eppy in jython once you remove the numpy dependency. Essentially once you are done with this and close this issue.

Vinc85 is wondering when that will happen :-) Seems to me that you are very close to being done

eayoungs commented 9 years ago

Yes, I was just looking into this. I was able to replace the remaining numpy dependencies with the Python native library, "math" so essentially I am done. I was working on a custom exception for singular matrices. Right now I've compromised and simply manually raised the base Exception class when the determinant is zero. I wonder if I/we should do some integration testing? vinc85 if you want to use the code, it should all be available in the most recent commit of the branch: "i30_pythongeometry"

vinc85: Great timing! This is quite validating and makes my work seem very useful. It's quite gratifying.

Cheers

Eric

santoshphilip commented 9 years ago

If it passes all the unit tests without installing numpy, you are done

eayoungs commented 9 years ago

Cool! I will set up a virtual environment and run the tests, then.

vinc85 commented 9 years ago

Many thanks Eric, I have tested eppy "i30_pythongeometry" and it seems to work without the library numpy. I hope it works now importing eppy in my java web application in Netbeans. I urge to run eppy on Web server.

 Il Martedì 16 Giugno 2015 1:27, Eric Youngson <notifications@github.com> ha scritto:

Cool! I will set up a virtual environment and run the tests, then.— Reply to this email directly or view it on GitHub.

santoshphilip commented 9 years ago

Also test if all the code in ./docs/Main_Tutorial.ipynb work. Open it up in ipython notebook and run all the cells.

vinc85 commented 9 years ago

Dear all,I have imported a new version of eppy (without numpy dependecies) in a Java Web Application and when I doing this iddfile = request.getContextPath() + '/Energy+V7_2_0.idd'fname1 = request.getContextPath() + '/smallfile.idf'IDF.setiddname(iddfile)idf1 = IDF(fname1) I have this error: Traceback (most recent call last): File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/serv.py", line 18, in doPost idf1 = IDF(fname1) File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/eppy/modeleditor.py", line 615, in init super(IDF3, self).init(idfname) File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/eppy/modeleditor.py", line 560, in init super(IDF2, self).init(idfname) File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/eppy/modeleditor.py", line 488, in init super(IDF1, self).init(idfname) File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/eppy/modeleditor.py", line 442, in init self.read() File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/eppy/modeleditor.py", line 472, in read readout = idfreader1(self.idfname, self.iddname, File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/eppy/idfreader.py", line 148, in idfreader1 versiontuple = iddversiontuple(iddfile) File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/eppy/idfreader.py", line 35, in iddversiontuple line1 = fhandle.readline() AttributeError: 'unicode' object has no attribute 'readline' Why? Many thanks.

 Il Mercoledì 17 Giugno 2015 1:44, santoshphilip <notifications@github.com> ha scritto:

Also test if all the code in ./docs/Main_Tutorial.ipynb work. Open it up in ipython notebook and run all the cells.— Reply to this email directly or view it on GitHub.

santoshphilip commented 9 years ago

vinc85, I have reformated your reply to make it more readable. see https://guides.github.com/features/mastering-markdown/ on how to do this

vince85's reply reformatted

Dear all,I have imported a new version of eppy (without numpy dependecies) in a Java Web Application and when I doing this

iddfile = request.getContextPath() + '/Energy+V7_2_0.idd'
fname1 = request.getContextPath() + '/smallfile.idf'
IDF.setiddname(iddfile)
idf1 = IDF(fname1)

I have this error:

Traceback (most recent call last):
  File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/serv.py", line 18, in doPost
    idf1 = IDF(fname1)
  File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/eppy/modeleditor.py", line 615, in __init__
    super(IDF3, self).__init__(idfname)
  File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/eppy/modeleditor.py", line 560, in __init__
    super(IDF2, self).__init__(idfname)
  File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/eppy/modeleditor.py", line 488, in __init__
    super(IDF1, self).__init__(idfname)
  File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/eppy/modeleditor.py", line 442, in __init__
    self.read()
  File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/eppy/modeleditor.py", line 472, in read
    readout = idfreader1(self.idfname, self.iddname,
  File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/eppy/idfreader.py", line 148, in idfreader1
    versiontuple = iddversiontuple(iddfile)
  File "/Users/vincenzo/NetBeansProjects/ProvaPython/build/web/eppy/idfreader.py", line 35, in iddversiontuple
    line1 = fhandle.readline()
AttributeError: 'unicode' object has no attribute 'readline'
santoshphilip commented 9 years ago

vinc85, That looks like a bug to me. I'll try to fix it in this branch

vinc85 commented 9 years ago

Ok, but I have not this error if I run eppy on desktop and not on a Web Application?Why? Many thanks.

 Il Mercoledì 17 Giugno 2015 16:48, santoshphilip <notifications@github.com> ha scritto:

vinc85, That looks like a bug to me. I'll try to fix it in this branch— Reply to this email directly or view it on GitHub.

santoshphilip commented 9 years ago

It works if the filename is a string Does not work if the filename is in unicode see here about string and unicode: https://docs.python.org/2/howto/unicode.html

java is sending the filename in unicode, while the desktop script is sending it in string

eppy should work with unicode too. This needs to be fixed.

vinc85 commented 9 years ago

It's true!!! ;-)

 Il Mercoledì 17 Giugno 2015 17:06, santoshphilip <notifications@github.com> ha scritto:

It works if the filename is a string Does not work if the filename is in unicode see here about string and unicode: https://docs.python.org/2/howto/unicode.htmljava is sending the filename in unicode, while the desktop script is sending it in stringeppy should work with unicode too. This needs to be fixed.— Reply to this email directly or view it on GitHub.

santoshphilip commented 9 years ago

vince85, try it now.

eayoungs (Eric), unit test is failing if I run it with numpy installed. Can you look into this ?

Santosh

vinc85 commented 9 years ago

I don't understand...

 Il Mercoledì 17 Giugno 2015 17:52, santoshphilip <notifications@github.com> ha scritto:

vince85, try it now. eayoungs (Eric), unit test is failing if I run it with numpy installed. Can you look into this ?Santosh— Reply to this email directly or view it on GitHub.

santoshphilip commented 9 years ago

I fixed the bug. Download and try again. Your code should work.

My other comment was directed to Eric. I does not affect you.

Santosh.

Sent from my iPad

On Jun 17, 2015, at 9:00 AM, vinc85 notifications@github.com wrote:

I don't understand...

Il Mercoledì 17 Giugno 2015 17:52, santoshphilip notifications@github.com ha scritto:

vince85, try it now. eayoungs (Eric), unit test is failing if I run it with numpy installed. Can you look into this ?Santosh— Reply to this email directly or view it on GitHub.

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

vinc85 commented 9 years ago

Ok, many thanks ;-)

Vincenzo

eayoungs commented 9 years ago

Sure Santosh; I will take a look. I was having some trouble with my virtual environment yesterday but I should have it figured out soon.

eayoungs commented 9 years ago

Santosh, All the tests for the branch seem to work with and without numpy installed on my machine (see below for details). Can you tell me more about which unit test fails and which packages are installed?

Thx!


(MacBook Pro 2014, OSX Yosemite / Python 2.7.10, virtualenv 13.03) $ pip list: beautifulsoup4 (4.3.2) bunch (1.0.1) eppy (0.4.6.4a0, /Users/eayoungs/repo/Code/SimTools/eppy) pip (7.0.3) py (1.4.28) pydot (1.0.2) pyparsing (1.5.7) pytest (2.7.1) setuptools (17.0) wheel (0.24.0)

santoshphilip commented 9 years ago

I'll double check on this. I assume we are speaking of branch "i30_pythongeometry"

santoshphilip commented 9 years ago

with numpy installed and running py.test I am getting

    def angle2vecs(vec1,vec2):
        # vector a * vector b = |a|*|b|* cos(angle between vector a and vector b)
        dot = np.dot(vec1,vec2)
        vec1_modulus = math.sqrt((vec1*vec1).sum())
        vec2_modulus = math.sqrt((vec2*vec2).sum())
        if (vec1_modulus * vec2_modulus) == 0:
            cos_angle = 1
        else: cos_angle = dot / (vec1_modulus * vec2_modulus)
>       return math.degrees(math.arccos(cos_angle))
E       AttributeError: 'module' object has no attribute 'arccos'

eppy/geometry/surface.py:95: AttributeError
santoshphilip commented 9 years ago

I am on branch i30_pythongeometry commit is bf7c05b60c6d1dbf97bc1a3e9ed754f5a99de160 This is the latest commit on that branch . It gave the above error

I also tested commit 1cc64fa0702340c092d115bd7b03725008523156 , the last commit you made on that branch. It gave the same error

Are we both testing the same commit ?

santoshphilip commented 9 years ago
pip install yolk

in your virtualenv.

if you type

yolk -l

it will list all the packages installed in that virtualenv

santoshphilip commented 9 years ago

I think numpy is not installed in your test where you think it is installed. You can also check this running the following from the shell

python -c 'import numpy'

if numpy is not installed, it will throw an ImportError

eayoungs commented 9 years ago

Mmm, yes. This is an oversight on my part. It should be math.acos. I fix this and check for any more issues.

eayoungs commented 9 years ago

I simply used a try/except pattern to import either numpy or, if not available, py_numeric to substitute but, on closer inspection, it looks like I might have to come up with a better way to replace numpy function calls within surface.py since the determinant function is stored another level down in the linalg module: np.linalg.det()

Also with the functions available from the math and array libraries, I simply renamed the function calls that are available in the math.py library but I realize that this may not be the best strategy if numpy is faster, then it should be used when available.

I'm wondering if we will need to have two different modules for each case and switch them when called elsewhere in the program. This doesn't seem ideal, though.

Any thoughts on how to abstract the function calls so within the module?

santoshphilip commented 9 years ago

Eric, Right now you are doing:

try:
    import numpy as np
except ImportError as e:
    import py_numeric as np

would the following solve the problem ?

try:
    import numpy as np
    import numpy.arccos as arccos # do same for other functions you need
except ImportError as e:
    import py_numeric as np
    import math.acos as arccos # do same for other functions you need
eayoungs commented 9 years ago

This does help. Although the issue still remains with the linalg submodule. I've moved the determinant function into a separate module to mimic the structure and put the import statement into the try/except pattern as you described above and it seems to work as far as that goes, but I've discovered a new issue.

I designed the determinant function to take 3 arguments as vectors but it looks like numpy takes a list of lists containing the vectors as a single parameter. It should be strait-forward to rewrite and parse the incoming list.

eayoungs commented 9 years ago

On another note, I found an interesting library that may help with limiting numpy dependecies, https://github.com/wadetb/tinynumpy I think it only replaces the ndarray class but it may come in handy.

eayoungs commented 9 years ago

I'm running into a type error with a particular unit test: eppy/tests/geometry_tests/test_surface.py - test_tilt() > tilt(poly) > unit_normal() > np.linalg.det()

The test returns 0, correctly but the numpy.float64 type passes with a warning while all the types I can access produce a divide by zero error. I can continue to reproduce as much of the determinant function as possible but it starts to get complicated, quickly and the core function seems to be implemented in C. I've pulled out the relevant code from the failing test and put it into an IPython notebook to introduce some print statements for debugging (possibly a case for logging). You can see the code here: https://gist.github.com/eayoungs/1c60eb2eb09de812bd4f

jamiebull1 commented 9 years ago

_edit to add: I've just pushed the fix below to the i30_pythongeometry branch you're working on if you want to pull it down._

Ah, that's actually my code that Sanjay Santosh dug up on ActiveState before I'd even heard of Eppy. The solution is, in eppy.geometry.surface.py, to explicitly test for the case where the points are all in line. The easiest way to do this is to test after the for loop in area(poly). It's also more efficient to test after the calculation rather than test for colinearity beforehand, given that it's probably rare that the function will be passed vertices that are in a straight line.

def area(poly):
    """Area of a polygon poly""" 
    if len(poly) < 3: # not a plane - no area
        return 0
    total = [0, 0, 0]
    N = len(poly)
    for i in range(N):
        vi1 = poly[i]
        vi2 = poly[(i+1) % N]
        prod = np.cross(vi1, vi2)
        total[0] += prod[0]
        total[1] += prod[1]
        total[2] += prod[2]
    if total == [0, 0, 0]:  # points are in a straight line
        return 0
    result = np.dot(total, unit_normal(poly[0], poly[1], poly[2]))
    return abs(result/2)
santoshphilip commented 9 years ago

That is indeed Jamie's code I used without attribution (since I did not know he wrote it) So in a sense Jamie was already a collaborator on eppy. (and somehow I became Sanjay instead of Santosh :-)

Eric, I am not paying too much attention to the specifics here. Will do so if you need me to. (all is well as long as unit tests pass at some point)

jamiebull1 commented 9 years ago

Haha, sorry about that. Now I understand why people never notice when they call me Jaime!

eayoungs commented 9 years ago

Thanks so much! I'm glad I asked. :-) Wait, now I'm confused; your name isn't Jaimie?

No worries Santosh; I'm on task now.

jamiebull1 commented 9 years ago

Jamie. But living in Spain, I've got quite used to Jaime. Jaimie is a new one though!

eayoungs commented 9 years ago

I've got all the tests passing now. I have a lot of cleaning up I'd like to do still but it seems to be working. I replaced the array class with a small open source project called tinynumpy and I'd like to implement all the functions I've created within that module for clarity and so I can contribute the code back to that project.