Skip to content

Feature/smarter array#40

Closed
CFlaniganMide wants to merge 16 commits intodevelopfrom
feature/smarter_array
Closed

Feature/smarter array#40
CFlaniganMide wants to merge 16 commits intodevelopfrom
feature/smarter_array

Conversation

@CFlaniganMide
Copy link
Copy Markdown
Contributor

Still some work left to be done, but here's an overview of what's been finished so far.

  • Eager-caching on document loading
    • TODO: change this to lazy-caching
    • TODO: remove/rework timestamp caching
  • rewrite arraySlice in EventArray to more effectively utilize the cached data
    • TODO: create rawArraySlice and calibrateRawArray methods which will reduce computation even further.
  • added inPlace transform methods which will, when possible, re-use memory.
    • goal: expand all transforms to generate a single statement. This will drastically reduce memory overhead.
  • rewrote some unit tests that had been skipped and now properly utilize an EventArray
    • goal: refactor that test suite - outside the scope of this PR

@StokesMIDE
Copy link
Copy Markdown
Member

Will this approach work with asynchronous loading, as done in the Lab?

@CFlaniganMide
Copy link
Copy Markdown
Contributor Author

Will this approach work with asynchronous loading, as done in the Lab?

It needs more work, but it should be fairly easy. How does the asynchronicity work in Lab? Are the WX callbacks different threads?

@StokesMIDE
Copy link
Copy Markdown
Member

Will this approach work with asynchronous loading, as done in the Lab?

It needs more work, but it should be fairly easy. How does the asynchronicity work in Lab? Are the WX callbacks different threads?

While loading, the length of an EventArray constantly changes, which makes caching things slightly tricky. The Lab does its importing in another thread, and it's the thread that concerns me; it's not specific to wxPython.

@CFlaniganMide
Copy link
Copy Markdown
Contributor Author

I think the way I would choose to do it would be to use a lock so that if the file is still loading (but not caching) it would have to wait. Here's basically how I would want to change it:

  • Add a lock to protect data access.
  • While loading, this lock would be held by the load function. Data is not cached yet.
  • When accessing the data, the accessing method would signal the first and last block of data it wants to access, then acquire the lock (with a timeout).
  • If the data is not cached, cache blocks which are not cached yet.
  • Grab a view of the underlying array with the correct indexing and generate an f64 copy.
  • Release the lock.
  • Continue as before.

This should be a fast and thread-safe way to access the data.

Comment thread idelib/dataset.py

def _initializeCache(self, *args, **kwargs):
""" Creates a cache of the raw data, either in memory or on disk """
from .parsers import ChannelDataArrayBlock
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is probably best to avoid imports within functions/methods unless there's a special reason for it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good catch

Comment thread idelib/dataset.py
and self.allowMeanRemoval == other.allowMeanRemoval

def _initializeCache(self, *args, **kwargs):
raise NotImplemented("EventArrays don't support caching")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since this isn't a method called by a user, would this be better as a warning?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I should probably just have it refer up to the parent instead of raising an error. I'm going to be reworking the caching soon-ish so it may become moot.

…uff and update old x-unit unittest code to pytest
@CFlaniganMide
Copy link
Copy Markdown
Contributor Author

caching is now sufficiently implemented in #49 to remove this PR

@CFlaniganMide CFlaniganMide deleted the feature/smarter_array branch August 3, 2021 20:55
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.

2 participants