CenterForTheBuiltEnvironment / pythermalcomfort

Package to calculate several thermal comfort indices (e.g. PMV, PPD, SET, adaptive) and convert physical variables.
https://pythermalcomfort.readthedocs.io/en/latest/
MIT License
144 stars 52 forks source link

utci numpy #24

Closed larsbuntemeyer closed 2 years ago

larsbuntemeyer commented 2 years ago

I would like to have comfort indicators work on numpy array. Here i start with utci:

to do:

It all should still work with floats but now also with numpy arrays (and much faster than using np.vectorize)

closes #23

larsbuntemeyer commented 2 years ago

i am not sure why the tests fail, seems unrelated to these changes here. can i run the travis ci on the pull request?

larsbuntemeyer commented 2 years ago

thanks for looking at this @FedericoTartarini !

FedericoTartarini commented 2 years ago

Thank you so much for working on this and for your help. If you have time I would like to implement the same also for the other models such as the PMV, SET,... since this is a great feature that you have implemented. Please let me know if you would have a bit of spare time to work on it. I will pull the changes locally, check that everything is fine and then I will release a new version of pythermalcomfort.

larsbuntemeyer commented 2 years ago

Yes, i will check out the other models as soon as possible. Mostly, it's does not require many change if you have some experience with numpy! I will also add the notebook to the examples!

FedericoTartarini commented 2 years ago

Great, yes please add the notebook to the examples since it would be super valuable to other users. If you want I can also do it. Let me try to implement what you have done for the UTCI inside the pmv_ppd function so I learn something new and I will then get back to you if I face some issues implementing it.

Thank you again for your work. It is now included inside the new pythermalcomfort version. Any other feedback you have about the tool would be highly appreciated.

FedericoTartarini commented 2 years ago

I have been working on converting the PMV and SET function to accept a NumPy array as input but I experienced some issues.

Inside the PMV and SET functions there are a lot of while and if loops. I converted the if -> np.where as you did, but I do not know how to handle the while loops (for example) and this seems to be causing some issues. Do you happen to know how to solve the problem?

Shall I use nditer

Moreover, rather than having to re-write the whole code, would it not be better to just check as soon as the function is called if the user passed an array as input. If that is the case, then we just call the existing functions passing a single element rather than an array. What do you think?

larsbuntemeyer commented 2 years ago

I have been working on converting the PMV and SET function to accept a NumPy array as input but I experienced some issues.

Inside the PMV and SET functions there are a lot of while and if loops. I converted the if -> np.where as you did, but I do not know how to handle the while loops (for example) and this seems to be causing some issues. Do you happen to know how to solve the problem?

Is that a kind of iterative solver (pmv_ppd_optimized)? I'll check this out, maybe there is some clever way of doing vectorized iterations in numpy...

Shall I use nditer

I think, for a start, you could check out np.vectorize, it's a simple way of applying a function on all array elements. However, this can become slow, if the function itself is not vectorized...

grafik

However, for a start, this could be a solution to apply pmv_ppd_optimized to an array...

Moreover, rather than having to re-write the whole code, would it not be better to just check as soon as the function is called if the user passed an array as input. If that is the case, then we just call the existing functions passing a single element rather than an array. What do you think?

That could probably be an intermediate solution? I just think that could produce some code duplication.

Should we make this a new issue?

FedericoTartarini commented 2 years ago

pmv_ppd_otimized is just a normal python function that is wrapped with Numba to significantly improve its performances.

At this stage, speed is not a major concern since all the functions are already optimized with Numba. Yes, both np.vectorize and np.nditer should both work fine. In addition, if we use this solution we do not need to change the functions themselves and we can keep them more readable. What do you mean with "if the function is not vectorized?"

I would not want to duplicate any functions since that would make the code very difficult to maintain.

Yes, we can open a new issue. We could also look into how other packages, such as scikit learn are handling vectors as inputs.