azogue / psychrochart

A Python 3 library to make psychrometric charts and overlay information on them.
MIT License
100 stars 26 forks source link

Psychrometric routines with PsychroLib #7

Closed dmey closed 4 years ago

dmey commented 4 years ago

Hi @azogue, well done for developing psychrochart!

A while ago we developed PsychroLib (https://github.com/psychrometrics/psychrolib) -- a library of psychrometric functions to calculate thermodynamic properties of air for Python, C, Fortran, JavaScript and VBA/Excel. I have noticed that you are maintaining several psychrometric functions yourself and, just in case you did not know, you may find the project useful to rely on PsychroLib rather than maintaining your own...

azogue commented 4 years ago

Hi @dmey

I didn't know about PsychroLib, but it looks very nice from a quick view :) Thanks for pointing it.

Yeah, it would be a good idea to externalize the psychrometric equations to a better, well-maintained repo, of course. And it would be very interesting to compare performances (PsychroLib's base algorithms are written in C or Fortran, right?)

Sadly, I have very little free time lately, so I don't know when I could try that change. Perhaps next month I could get some time, but can't be sure. Anyway, if you want to try it and make a MR, please be my guest :)

dmey commented 4 years ago

Thanks. The libraries are native for all the languages we support (Python, C, Fortran, JS, and VBA) so in your case, since you use Python, you could simply depend on it -- i.e. import PsychroLib having it previously installed from PiPy (see https://pypi.org/project/PsychroLib/) -- I guess that if the API is the same/similar this wouldn't be much work (you can see the API docs at https://psychrometrics.github.io/psychrolib/api_docs.html). With regards to testing we carry out several unit-tests across the different implementations at every commit using CI (see https://travis-ci.com/psychrometrics/psychrolib/jobs/200381713) we have also compared some against EnergyPlus.

I am also swamped with work, I am afraid, so I cannot commit to to submitting a PR at the moment... This is nothing critical though and I guess can be done when either of us find the time/need to do it...

zha commented 4 years ago

@dmey I have been working on it in the last few days and it should be done in the next month or so.

dmey commented 4 years ago

@zha how are you getting on with this?. We have recently make a few imporvments and bugfixes and added .NET support (C#) see https://github.com/psychrometrics/psychrolib.

zha commented 4 years ago

Dear @dmey,

Thanks for the update. I have spent time to work on it and it worked for the first step in this process. However, I still need to keep working on it since I need to make sure that everything is integrated. I have not been able to get back on it but hopefully I will be able to continue work on it and finalize it during the Christmas time. Hope that is fine with you.

Thanks,

Shen

dmey commented 4 years ago

Sure @zha -- I'll let you know if we may any more changes. Dec/Jan should hopefully be easier with me too in case you need any help 😃.

azogue commented 4 years ago

Hi @dmey, @zha,

Sorry for the absence :(

I had some time to revisit this repository, and I did some code cleaning and also the psychrolib integration ;-)

As the objective of this repo is not to maintain a good collection of equations (that is very good accomplished in psychrolib), but just to provide an easy way to plot psychrometric charts, I'll focus on that and remove all equations from here.

I'll push some merge requests in this direction, and I hope with those changes future improvements will be easier to implement (like supporting imperial units out-of-the-box)

I will link the changes to this conversation so you can quickly review the changes, have a good day :)

dmey commented 4 years ago

Great @azogue 👍.

azogue commented 4 years ago

Hi, integration with psychrolib is already done in #9, also with minor changes to plot charts in IP units.

I've done a quick test and result is not bad, I think:

test_ashrae_psychrochart_ip

So after merging those changes and publish a new version in pypi, I will consider this issue closed, ok @dmey ?

BTW, it would be nice to point out this repo as a usage example for psychrolib, in its documentation :)

Anyway, I also want to suggest a couple of methods more to the python library of psychrolib, as I needed them but they weren't there. I put them in a separated module .psychrolib_extra, using the same code style that it's used in that repo, and I will be posting a pull request there asap, if you don't mind.

Have a good weekend :)

dmey commented 4 years ago

@azogue looks good -- I'll have a look at the #9 in the next couple of days and will let you know. Re https://github.com/psychrometrics/psychrolib/pull/44 thanks -- I'll work on this first sometime this weekend and will let you know once done so that you can just rely on the release on PyPi but let me know if this is urgent and I will prioritise it over other stuff.

azogue commented 4 years ago

Thanks @dmey, there is no hurry, so take your time.

I also want to update the readme before merging #9, to point there to psychrolib and add some comment about SI/IP units. In next PRs I will try to improve the chart config interface and maybe add some predefined styles for IP charts

azogue commented 4 years ago

There is already a version 0.3 in pypi, as the product of the un-merged #11, which uses vectorized psychrolib and can plot in IP units :)