Skip to content

Fetch hk#771

Open
sanahabhimani wants to merge 12 commits intomasterfrom
fetch_hk
Open

Fetch hk#771
sanahabhimani wants to merge 12 commits intomasterfrom
fetch_hk

Conversation

@sanahabhimani
Copy link
Copy Markdown
Contributor

This is a working version for speeding up hk loading using .g3 frames.

Args are path to HK .g3 filename and a list of fields. If fields is None, data for all fields in that .g3 file are returned. Skips any address in .g3 frames that has daq-registry in it.

@sanahabhimani sanahabhimani self-assigned this Mar 18, 2024
@kmharrington
Copy link
Copy Markdown
Member

@jlashner could you take a look at this?

@mhasself
Copy link
Copy Markdown
Member

mhasself commented Mar 20, 2024

Suggestions:

  • can this go in hk_utils.py?
  • instead of "fetch_hk", pick a function name that describes why one might use this loader instead of another. "quick_read_hk"?
  • your big lists should be converted to np.arrays before returning to the user.
  • instead of converting timestamps one by one, do them as a batch after everything is collected. It will be faster.
  • you should probably watch for name collisions -- e.g. if you end up storing "temperature" from two different agents, in the same vector, the results will be pretty difficult to interpret and debug. whoops, nevermind ... misread something.
  • I don't think lines 44-45 are needed; if they are then explain in comment.

I know this is answering a need for a faster way to do something. Loading HK in a single pass is going to be the fastest. But I think someone should take a high-level look at the HK problem... I am concerned that we are getting new ways of loading HK all the time, without necessarily converging on a single better way.

@sanahabhimani
Copy link
Copy Markdown
Contributor Author

Did we decide whether this tool is still valuable?

@jlashner
Copy link
Copy Markdown
Contributor

I know this is something we've discussed before, but looking at the code now I'm left a little confused as to what the use case is. Namely, how do users know what g3 files they need to be scanning? Are there other functions in hk_util that give you the files to scan for a given time range and is this addressing a particular inefficiency in load_range?

I think since we're on the verge of getting hkdb instances running, I think if we can't think of clear use cases, I'm tempted to say we close this to avoid adding more confusion as to which hk loading function does what.

@sanahabhimani
Copy link
Copy Markdown
Contributor Author

I agree--I was pruning my old branches and draft PR's and when coming across this one, I couldn't justify a use case for this.

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.

4 participants