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

Patch progress bar #276

Merged
merged 16 commits into from Jul 21, 2020
Merged

Conversation

WenzDaniel
Copy link
Collaborator

@WenzDaniel WenzDaniel commented Jul 1, 2020

With this pull request we add a nice progessbar. Implemented in get_iter. It also works if the user specifies any time range.

example

@WenzDaniel WenzDaniel mentioned this pull request Jul 1, 2020
@JoranAngevaare
Copy link
Member

Hi Daniel, would it be an option to make the progress-bar optional? Like e.g. https://github.com/JelleAalbers/wimprates/blob/master/wimprates/utils.py#L44.

@WenzDaniel
Copy link
Collaborator Author

Hi Joran, thanks. I have to check.

@WenzDaniel
Copy link
Collaborator Author

Hi Daniel, would it be an option to make the progress-bar optional? Like e.g. https://github.com/JelleAalbers/wimprates/blob/master/wimprates/utils.py#L44.

Done, the bar is now optional. By default the bar is switched on. Also added a check that the progress bar is only used if metadata exists.

@JoranAngevaare
Copy link
Member

@WenzDaniel some extra questions:

  • Do you know how much extra time we need for a typical run to load the data (and does it differ much for event_info / raw_records)?
  • If so perhaps we can make the default different for get_array/get_df might be an option.
  • Do you know if it also works for superruns or should we always force it to be false there?

strax/context.py Show resolved Hide resolved
strax/context.py Outdated Show resolved Hide resolved
@WenzDaniel
Copy link
Collaborator Author

WenzDaniel commented Jul 9, 2020

@WenzDaniel some extra questions:

* Do you know how much extra time we need for a typical run to load the data (and does it differ much for event_info / raw_records)?

To be honest I never really checked, since the overall added extra time is super small compared to all the other checks we are doing when reading data. I expect the time difference to be very small. I guess the part which takes longest is the reading in of the metadata for which I have not seen any big differences for the different datakinds.

Update:

I just checked with

%%timeit
for i in st.get_iter(run_id_led, 'raw_records', seconds_range=(0, 30), progress_bar=True):
    pass

for a typical calibration run and I got the following values:

  • 1min 44s ± 1.33 s
  • 1min 47s ± 2.76 s

So there seems to be an overhead O(1 s) which I think is fine.

* Do you know if it also works for superruns or should we always force it to be false there?

No, I never worked with superruns and I do not know how they work. If it is a nested loop for run in runid -> for .. in get_iter -> you will get for each run an individual progress bar, if you first somewhat make a "super run" for which you loop just once with get_iter you will get just a single bar.

** Update: **

I just read through https://github.com/AxFoundation/strax/blob/master/strax/run_selection.py#L221 and since super runs have the same structure as regular runs including a metadata file the progess bar should also work for them.

@JoranAngevaare
Copy link
Member

@ ACs this is ready for merge unless @WenzDaniel feels like addressing CodeFactor

@JoranAngevaare
Copy link
Member

@WenzDaniel , this PR would be great but I cannot circumvent the codefactor

@JoranAngevaare JoranAngevaare merged commit b46efae into AxFoundation:master Jul 21, 2020
@WenzDaniel WenzDaniel deleted the patch_progress_bar branch September 2, 2020 23:24
@JoranAngevaare JoranAngevaare linked an issue Oct 15, 2020 that may be closed by this pull request
@JoranAngevaare JoranAngevaare linked an issue Nov 20, 2020 that may be closed by this pull request
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.

Move progressbar from get_iter Error in progres_bar
2 participants