EnviroDIY performance and related issues - Githubissues

ODM2 / WOFpy

A server-side implementation of CUAHSI's Water One Flow service stack in Python.

http://odm2.github.io/WOFpy/

9 stars 9 forks source link

EnviroDIY performance and related issues #228

Open emiliom opened 6 years ago

emiliom commented 6 years ago

Compiling in one place (to make it easier to follow up) various issues and investigation that have arisen in testing EnviroDIY WOFpy performance on the Model-My-Watershed App.

https://github.com/ODM2/ODM2DataSharingPortal/issues/258 Where the main relevant issues and findings have been discussed.
https://github.com/WikiWatershed/model-my-watershed/pull/2840 PR with important discussions, related to the above issue, I think.
https://github.com/WikiWatershed/model-my-watershed/issues/2859 This PR is now closed, but there may be relevant outstanding issues/discussions in there.
https://github.com/ulmo-dev/ulmo/issues/155 Issue submitted to ulmo with clear description of recommended enhancement to WOF reader. See also https://github.com/WikiWatershed/model-my-watershed/issues/2807 (directly related to the ulmo issue, I think)
224 is an issue here that compiled discussions and decisions from the initial EnviroDIY WOFpy deployment and its endpoint registration at CUAHSI. Anthony closed it in late July. If there's time, go over it to see if there's anything of ongoing value there.

emiliom commented 6 years ago

ulmo: issues to address in a PR (or more) to ulmo

Won't fix at this time, 1/8/2019. See comment below. From https://github.com/WikiWatershed/model-my-watershed/issues/2807#issuecomment-392942792: "Timestamp().to_datetime was deprecated in Pandas 0.21 and removed in 0.23. Since Ulmo does not specify dependency versions, the latest version of Pandas is used, causing this error. It would be great to get this fixed on the Ulmo end. In the interim I'll see if Pandas can be pinned to an older version that still has .to_datetime support." The failure is "happening at this line in Ulmo.
- From Pandas doc: "Timestamp.to_datetime(): DEPRECATED: use to_pydatetime() instead. Convert a Timestamp object to a native Python datetime object."
- See if there's any linkage to this old (2017-8), merged ulmo commit or later PR's, such as https://github.com/ulmo-dev/ulmo/pull/157 (also mentioned in https://github.com/ulmo-dev/ulmo/issues/160)
- See if functionality from util/misc.py can be leveraged; eg, https://github.com/ulmo-dev/ulmo/blob/master/ulmo/util/misc.py#L48. ACTUALLY, this is the line that Azavea found to be failing!!
- Actually, on 2017-10-5 the ulmo conda_environment.yml was pinned to pandas<=0.22.0 in https://github.com/ulmo-dev/ulmo/commit/a324fc0149839ab52e0a4009662ca32827c42c3e
Submitted PR on 1/7/2019. https://github.com/ulmo-dev/ulmo/issues/155 Issue submitted to ulmo with clear description of recommended enhancement to WOF reader: "need to specify a custom timeout (as opposed to the default 90s). suds-jurko supports timeouts for both the http and https modules. It would be great if they could be surfaced in the Ulmo API, just as caching currently is. Without it, we have to wrap it in a timeout decorator, which is fine for now, but limits us to single-threaded work, since it cannot handle non-pickleable responses in multithreaded environments. This wrapping is extraneous since suds does have timeout support, it's just hidden from use."
- See this expanded description from Azavea
- Also see https://github.com/andersinno/suds-jurko/issues/3
Submitted PR on 1/7/2019. Tweak I made to ulmo many months ago (probably mid-late 2017), and kept on my personal fork but never submitted as PR back to ulmo. See my wml_values_md branch on my fork
Issued new ulmo release, 0.8.5, on 3/23-25/2019. This release incorporates all the issues discussed here and flagged as to-be-addressed. packages are available on conda-forge and pypi

emiliom commented 5 years ago

Submitted ulmo PR with the two updates involving waterml parsing and wof/suds timeout, https://github.com/ulmo-dev/ulmo/pull/165

I won't fix the other issue (pandas Timestamp().to_datetime) at this time, b/c the current ulmo conda_environment.yml was already pinned to pandas<=0.22.0.

emiliom commented 5 years ago

Link to the Python code in the MMW client that handles querying and reading CUAHSI HIS data services: https://github.com/WikiWatershed/model-my-watershed/tree/develop/src/mmw/apps/bigcz/clients/cuahsi ulmo WOF access is done here

emiliom commented 5 years ago

For reference re: database optimization strategies to explore: https://github.com/ODM2/ODM2-performance-optimization/issues/7

emiliom commented 5 years ago

Regarding https://github.com/ODM2/WOFpy/issues/228#issuecomment-452091892: my ulmo PR (plus many others) has been merged into ulmo master. A new release is being prepared, and will almost certainly be ready by mid February. A new conda package will be created on conda-forge immediately after.

aufdenkampe commented 5 years ago

@emiliom, thanks for moving the Ulmo PR forward into an upcoming Ulmo release. That is awesome.

emiliom commented 5 years ago

We've issued a new ulmo release, 0.8.5, with all recent bug fixes and improvements, including the ones discussed in this issue. It's available on conda-forge and pypi.