{"action":"created","issue":{"url":"https://api.github.com/repos/pvlib/pvlib-python/issues/416","repository_url":"https://api.github.com/repos/pvlib/pvlib-python","labels_url":"https://api.github.com/repos/pvlib/pvlib-python/issues/416/labels{/name}","comments_url":"https://api.github.com/repos/pvlib/pvlib-python/issues/416/comments","events_url":"https://api.github.com/repos/pvlib/pvlib-python/issues/416/events","html_url":"https://github.com/pvlib/pvlib-python/issues/416","id":293629981,"number":416,"title":"inconsistent broadcasting of outputs from singlediode methods","user":{"login":"mikofski","id":1385621,"avatar_url":"https://avatars1.githubusercontent.com/u/1385621?v=4","gravatar_id":"","url":"https://api.github.com/users/mikofski","html_url":"https://github.com/mikofski","followers_url":"https://api.github.com/users/mikofski/followers","following_url":"https://api.github.com/users/mikofski/following{/other_user}","gists_url":"https://api.github.com/users/mikofski/gists{/gist_id}","starred_url":"https://api.github.com/users/mikofski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mikofski/subscriptions","organizations_url":"https://api.github.com/users/mikofski/orgs","repos_url":"https://api.github.com/users/mikofski/repos","events_url":"https://api.github.com/users/mikofski/events{/privacy}","received_events_url":"https://api.github.com/users/mikofski/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":6,"created_at":"2018-02-01T18:06:34Z","updated_at":"2018-02-15T00:46:12Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"The issue was raised in [some comments](./410#issuecomment-361974051) in #410 and #409 that the rules for broadcasting outputs are inconsistent among the single diode methods\r\n\r\n* `singlediode` will only broadcast the output if `photocurrent` is an sequence, array or series\r\n    - float -> OrderedDict of float\r\n    - array or sequence -> OrderedDict of array or sequence\r\n    - Series -> DataFrame\r\n* `i_from_v` will broadcast the smallest size of either `voltage`, `photocurrent`, or `resistance_shunt`\r\n* `v_from_i` will broadcast the smallest size of either `current`, `photocurrent`, or `resistance_shunt`\r\n\r\nIn those same comments @cwhanse proposed:\r\n> to expand singletons when any parameter input is a vector, and to fail (gracefully) if two parameters are vectors of unequal length\r\n\r\nI agree with this idea, and both `i_from_v` and `v_from_i` already have some boilerplate code to address this (note this snippet is lifted from `v_from_i`, with variable names suited for that function, change the\r\narguments to other methods like singlediode or mpp):\r\n\r\n        # find the right size and shape for returns\r\n        args = (current, photocurrent, saturation_current,\r\n                resistance_series, resistance_shunt, nNsVth)  <-- list the args allowed to broadcast here\r\n        size, shape = 0, None  # 0 or None both mean scalar\r\n        for arg in args:\r\n            try:\r\n                this_shape = arg.shape  # try to get shape\r\n            except AttributeError:\r\n                this_shape = None\r\n                try:\r\n                    this_size = len(arg)  # try to get the size\r\n                except TypeError:\r\n                    this_size = 0\r\n            else:\r\n                this_size = sum(this_shape)  # calc size from shape\r\n                if shape is None:\r\n                    shape = this_shape  # set the shape if None\r\n            # update size and shape\r\n            if this_size > size:\r\n                size = this_size\r\n                if this_shape is not None:\r\n                    shape = this_shape\r\n\r\nNow knowing the `size` and/or the `shape` one can let [`numpy.vectorize`](https://docs.scipy.org/doc/numpy/reference/generated/numpy.vectorize.html) cast the output as proposed:\r\n\r\n        if size <= 1:\r\n            V = v_from_i_fun(*args)\r\n            if shape is not None:\r\n                V = np.tile(V, shape)  <-- hacky way to make either array(V), array([V]), array([[V]]), ...\r\n        else:\r\n            # np.vectorize handles broadcasting, raises ValueError\r\n            vecfun = np.vectorize(v_from_i_fun)\r\n            V = vecfun(*args)\r\n        if np.isnan(V).any() and size <= 1:\r\n            V = np.repeat(V, size)\r\n            if shape is not None:\r\n                V = V.reshape(shape)\r\n\r\nOn a related note: @cwhanse I realize now I misread [your comment](./410#issuecomment-362112394) and made `i_from_v` and `v_from_i` fail _silently_ instead of _gracefully_, oops!. This might cause some confusion, in the short term, but maybe we can address it in this issue here?\r\n\r\n@wholmgren is there a `PVLibException` class? What exception would you expect here, maybe `ValueError('All arguments must be either scalar or the same size.')`?"},"comment":{"url":"https://api.github.com/repos/pvlib/pvlib-python/issues/comments/365793136","html_url":"https://github.com/pvlib/pvlib-python/issues/416#issuecomment-365793136","issue_url":"https://api.github.com/repos/pvlib/pvlib-python/issues/416","id":365793136,"user":{"login":"mikofski","id":1385621,"avatar_url":"https://avatars1.githubusercontent.com/u/1385621?v=4","gravatar_id":"","url":"https://api.github.com/users/mikofski","html_url":"https://github.com/mikofski","followers_url":"https://api.github.com/users/mikofski/followers","following_url":"https://api.github.com/users/mikofski/following{/other_user}","gists_url":"https://api.github.com/users/mikofski/gists{/gist_id}","starred_url":"https://api.github.com/users/mikofski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mikofski/subscriptions","organizations_url":"https://api.github.com/users/mikofski/orgs","repos_url":"https://api.github.com/users/mikofski/repos","events_url":"https://api.github.com/users/mikofski/events{/privacy}","received_events_url":"https://api.github.com/users/mikofski/received_events","type":"User","site_admin":false},"created_at":"2018-02-15T00:46:12Z","updated_at":"2018-02-15T00:46:12Z","author_association":"CONTRIBUTOR","body":"Sorry @wholmgren I think this is specific to #409 proposed methods that didn't seem to be broadcasting correctly?\r\n\r\nI've tested the existing methods in the master branch with different size inputs and they seem to broadcast okay, as long as numpy arrays are used. There does seem to be some preferential treatment for `photocurrent` as it can be a list or an array.\r\n\r\nHere are some examples:\r\n#### `singlediode` \r\n* list of photocurrents, all other inputs scalar - works!\r\n    ```singlediode([6, 7, 8], 1e-7, 0.001, 10, 1.23*0.026*72)```\r\n* same size arrays of photocurrents and shunt resistance, others scalar - works!\r\n    ```singlediode(np.array([6, 7, 8]), 1e-7, 0.001, np.array([10, 11, 12]), 2.3)```\r\n* array of shunt resistance, all others scalar - works!\r\n    ```singlediode(6, 1e-7, 0.001, np.array([10, 11, 12]), 2.3)```\r\n* same size arrays of shunt and series resistance, others scalar - works\r\n    ```singlediode(6, 1e-7, np.array([0.001, 0.002, 0.003]), np.array([10, 11, 12]), 2.3)```\r\n\r\n#### `i_from_v`\r\n* list of photocurrents, all other inputs scalar - works!\r\n    ```i_from_v(10, 0.001, 2.3, 0.5, 1e-7, [6, 7, 8])```\r\n* same size list of voltages and photocurrents, others scalar - works!\r\n    ```i_from_v(10, 0.001, 2.3, [0.4, 0.45, 0.5], 1e-7, [6, 7, 8])```\r\n* list of voltages, others scalar - works!\r\n    ```i_from_v(10, 0.001, 2.3, [0.4, 0.45, 0.5], 1e-7, 6)```\r\n* same size array of Rsh and voltages, others scalar - works!\r\n    ```i_from_v(np.array([10, 11, 12]), 0.001, 2.3, np.array([0.4, 0.45, 0.5]), 1e-7, 6)```\r\n\r\nI've change the title to make it specific to PR #409 "}}
{"actor":{"display_login":"mikofski"}}