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

Improve speed of getting local OCAT data #272

Merged
merged 3 commits into from
May 17, 2022
Merged

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented May 9, 2022

Description

Improve the speed of reading the local OCAT. Most of the time is spent decoding the UTF-8

Interface impacts

None.

Testing

Unit tests

  • Mac

Independent check of unit tests by Javier

Functional tests

Current release

(ska3) ➜  docs git:(ocat-local-faster) ipython
Python 3.8.12 (default, Oct 12 2021, 06:23:56) 
Type 'copyright', 'credits' or 'license' for more information
IPython 7.29.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from mica.archive.cda import get_ocat_local
In [2]: %time dat = get_ocat_local()
CPU times: user 1.69 s, sys: 83 ms, total: 1.77 s
Wall time: 1.77 s

New using current compressed HDF5

(ska3) ➜  mica git:(ocat-local-faster) ipython
Python 3.8.12 (default, Oct 12 2021, 06:23:56) 
Type 'copyright', 'credits' or 'license' for more information
IPython 7.29.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from mica.archive.cda import get_ocat_local
In [2]: %time dat = get_ocat_local()
CPU times: user 456 ms, sys: 93.3 ms, total: 549 ms
Wall time: 549 ms

New using uncompressed HDF5

In [2]: %time dat = get_ocat_local(datafile="ocat.h5")
CPU times: user 298 ms, sys: 103 ms, total: 401 ms
Wall time: 403 ms

# above 128 that signify a non-ASCII character.
itemsize = col.dtype.itemsize
col_bytes = col.view((np.uint8, (itemsize,)))
if np.all(col_bytes.flatten() < 128):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't np.all handle all shapes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, good point, will fix.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

mica/archive/cda/services.py Outdated Show resolved Hide resolved
# but with the single leading byte set.
col_utf8 = np.zeros((col_bytes.shape[0], itemsize * 4), dtype=np.uint8)
for ii in range(itemsize):
col_utf8[:, ii * 4] = col_bytes[:, ii]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this loop the same as this?

col_utf8_2[:,::4] = col_bytes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, but it requires a little thinking to be sure it will be right. Writing it out in a loop makes the intent blindingly obvious and is effectively just as fast (given all the other overhead).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this comment after testing that it was equivalent, so my question was a bit rhetorical, but ok.

@taldcroft taldcroft requested review from javierggt and jeanconn May 12, 2022 12:24
@taldcroft taldcroft force-pushed the ocat-local-faster branch from b04bd13 to db7b2e0 Compare May 14, 2022 09:51
@taldcroft
Copy link
Member Author

I fixed the thing about using an observer name, though that comments seems to be gone now.

Copy link
Contributor

@javierggt javierggt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did the following:

  • ran pytest and it was ok.
  • reproduced the one-line functional tests in the description.
  • made the change to use col_utf8[:,::4] = col_bytes instead of the loop and ran the on-line tests:
In [1]: from mica.archive.cda import get_ocat_local

In [2]: %time dat = get_ocat_local()
CPU times: user 400 ms, sys: 104 ms, total: 504 ms
Wall time: 509 ms

In [3]: %time dat = get_ocat_local(datafile="ocat.h5")
CPU times: user 265 ms, sys: 63.9 ms, total: 329 ms
Wall time: 328 ms

# but with the single leading byte set.
col_utf8 = np.zeros((col_bytes.shape[0], itemsize * 4), dtype=np.uint8)
for ii in range(itemsize):
col_utf8[:, ii * 4] = col_bytes[:, ii]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this comment after testing that it was equivalent, so my question was a bit rhetorical, but ok.

@taldcroft taldcroft merged commit 756a27c into master May 17, 2022
@taldcroft taldcroft deleted the ocat-local-faster branch May 17, 2022 15:43
@javierggt javierggt mentioned this pull request May 31, 2022
@javierggt javierggt mentioned this pull request Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants