Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Vectorised PropSI breaks plotting functions #387

Closed
jowr opened this issue Jan 2, 2015 · 14 comments
Closed

Vectorised PropSI breaks plotting functions #387

jowr opened this issue Jan 2, 2015 · 14 comments
Milestone
v5.0.7

Comments

@jowr
Copy link
Member

jowr commented Jan 2, 2015

Looks like the new PropsSI causes some trouble.

Traceback (most recent call last):
    isoqual = isoObj.get_isolines([0.0,1.0], num=2)
  File "/home/jowr/envs/CoolProp5/lib/python2.7/site-packages/CoolProp/Plots/Plots.py", line 370, in get_isolines
    lines = self._get_sat_lines(x=iso_range)
  File "/home/jowr/envs/CoolProp5/lib/python2.7/site-packages/CoolProp/Plots/Common.py", line 183, in _get_sat_lines
    kind, sat_mesh)
  File "/home/jowr/envs/CoolProp5/lib/python2.7/site-packages/CoolProp/Plots/Common.py", line 153, in _get_fluid_data
    return numpy.array([x_vals, y_vals])
ValueError: could not broadcast input array from shape (500,1) into shape (500)
@ibell
Copy link
Contributor

ibell commented Jan 2, 2015

Probably needs a squeeze() call to chop off useless axis. We might want to
do that automatically in pyx file
On Jan 2, 2015 1:58 PM, "Jorrit Wronski" notifications@github.com wrote:

Looks like the new PropsSI causes some trouble.

Traceback (most recent call last):
isoqual = isoObj.get_isolines([0.0,1.0], num=2)
File "/home/jowr/envs/CoolProp5/lib/python2.7/site-packages/CoolProp/Plots/Plots.py", line 370, in get_isolines
lines = self._get_sat_lines(x=iso_range)
File "/home/jowr/envs/CoolProp5/lib/python2.7/site-packages/CoolProp/Plots/Common.py", line 183, in _get_sat_lines
kind, sat_mesh)
File "/home/jowr/envs/CoolProp5/lib/python2.7/site-packages/CoolProp/Plots/Common.py", line 153, in _get_fluid_data
return numpy.array([x_vals, y_vals])
ValueError: could not broadcast input array from shape (500,1) into shape (500)

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

@jowr
Copy link
Member Author

jowr commented Jan 2, 2015

Go ahead and fix it 😉

@ibell
Copy link
Contributor

ibell commented Jan 3, 2015

Will do, I just need to figure out how best to handle it. I think the
squeeze() should do the trick

On Fri, Jan 2, 2015 at 2:08 PM, Jorrit Wronski notifications@github.com
wrote:

Go ahead and fix it [image: 😉]

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

jowr pushed a commit that referenced this issue Jan 4, 2015
This is a preliminary fix for the issue #387 because I need the code.…
c60ee1d 
… I hope this does not break things for others.
@jowr
Copy link
Member Author

jowr commented Jan 5, 2015

There are some more pitfalls hiding. Tv and Pv are numpy arrays of the same length with valid inputs:

Dv = PropsSI("D","T",Tv,"P",Pv,fluid)
File "CoolProp/CoolProp.pyx", line 204, in CoolProp.CoolProp.PropsSI (CoolProp/CoolProp.cpp:16510)
File "CoolProp/CoolProp.pyx", line 238, in CoolProp.CoolProp.PropsSI (CoolProp/CoolProp.cpp:15967)
File "stringsource", line 63, in vector.from_py.__pyx_convert_vector_from_py_double (CoolProp/CoolProp.cpp:25113)
TypeError: only length-1 arrays can be converted to Python scalars

Note that Tv and Pv are 2D arrays, if that is not supported, we should provide a proper error message.

@ibell
Copy link
Contributor

ibell commented Jan 6, 2015

Right, did 2D arrays ever work before?

On Mon, Jan 5, 2015 at 8:00 AM, Jorrit Wronski notifications@github.com
wrote:

There are some more pitfalls hiding. Tv and Pv are numpy arrays of the
same length with valid inputs:

Dv = PropsSI("D","T",Tv,"P",Pv,fluid)
File "CoolProp/CoolProp.pyx", line 204, in CoolProp.CoolProp.PropsSI (CoolProp/CoolProp.cpp:16510)
File "CoolProp/CoolProp.pyx", line 238, in CoolProp.CoolProp.PropsSI (CoolProp/CoolProp.cpp:15967)
File "stringsource", line 63, in vector.from_py.__pyx_convert_vector_from_py_double (CoolProp/CoolProp.cpp:25113)
TypeError: only length-1 arrays can be converted to Python scalars

Note that Tv and Pv are 2D arrays, if that is not supported, we should
provide a proper error message.

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

@ibell
Copy link
Contributor

ibell commented Jan 6, 2015

Ah, they are Nx1 in dimension, right?

On Mon, Jan 5, 2015 at 11:26 PM, Ian Bell ian.h.bell@gmail.com wrote:

Right, did 2D arrays ever work before?

On Mon, Jan 5, 2015 at 8:00 AM, Jorrit Wronski notifications@github.com
wrote:

There are some more pitfalls hiding. Tv and Pv are numpy arrays of the
same length with valid inputs:

Dv = PropsSI("D","T",Tv,"P",Pv,fluid)
File "CoolProp/CoolProp.pyx", line 204, in CoolProp.CoolProp.PropsSI (CoolProp/CoolProp.cpp:16510)
File "CoolProp/CoolProp.pyx", line 238, in CoolProp.CoolProp.PropsSI (CoolProp/CoolProp.cpp:15967)
File "stringsource", line 63, in vector.from_py.__pyx_convert_vector_from_py_double (CoolProp/CoolProp.cpp:25113)
TypeError: only length-1 arrays can be converted to Python scalars

Note that Tv and Pv are 2D arrays, if that is not supported, we should
provide a proper error message.

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

@jowr
Copy link
Member Author

jowr commented Jan 7, 2015

nope, they are truly 2d, guess we just need a better message.

@ibell
Copy link
Contributor

ibell commented Jan 7, 2015

Did that ever work before? I'm not sure why it would have... Your best
bet would be to reshape the inputs to Nx1, then reshape back after the
calculations

On Wed, Jan 7, 2015 at 2:56 PM, Jorrit Wronski notifications@github.com
wrote:

nope, they are truely 2d, guess we just need a better message.

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

@jowr
Copy link
Member Author

jowr commented Jan 7, 2015

Yep, it never worked before. I was just curious to test it now that we have the mighty PropsSImulti that works like a charm.

@ibell
Copy link
Contributor

ibell commented Jan 7, 2015

Unrelated, but have you done any speed comparisons between PropsSImulti and
multiple PropsSI calls? I'm curious how much of a difference it makes.

On Wed, Jan 7, 2015 at 3:09 PM, Jorrit Wronski notifications@github.com
wrote:

Yep, it never worked before. I was just curious to test it now that we
have the mighty PropsSImulti that works like a charm.

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

@jowr jowr mentioned this issue Jan 9, 2015
Closed
@ibell
Copy link
Contributor

ibell commented Jan 13, 2015

What should we do about this issue? Can you recommend a solution? I can implement it.

@jowr
Copy link
Member Author

jowr commented Jan 13, 2015

Well, I think it is fixed. If no one encountered any problems yet, my solution might just work (for once). Haven't tested the speed though.

@jowr
Copy link
Member Author

jowr commented Jan 13, 2015

We are still missing a propper error message though.

@ibell
Copy link
Contributor

ibell commented Jan 14, 2015

Hmm I can fix that. Should be easy to repair.

On Tue, Jan 13, 2015 at 1:10 AM, Jorrit Wronski notifications@github.com
wrote:

We are still missing a propper error message though.

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

@ibell ibell added this to the v5.0.7 milestone Jan 20, 2015
@ibell ibell closed this as completed in 06003c6 Jan 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v5.0.7
Development

No branches or pull requests
2 participants
@jowr @ibell