Skip to content

Conversation

@mpharrigan
Copy link
Collaborator

  • include job timestamp
  • re-think aggregation strategy

Background: the analysis code extracts the results into a pandas dataframe. You need to aggregate the quantities to make plots and create fits.

Previously: I knew additional columns (i.e. fields) would be added over time to the results dataframe. I thought I could be clever with the aggregation to make the addition of new fields automatic. This was done with the groupby_all_except function which inverts the api of groupby.

Then: we actually need a custom aggregation for the job_finished_time field. Every time the dataframe gets aggregated down, I've chosen to pick the "last" time of the group. You can imagine choosing the first or average. In any event: the groupby_all_except approach didn't make this magically work. One still needs to plumb through the new field with care.

This PR: make our groupby columns and the aggregation functions / output columns explicit everywhere. Going forward, this shouldn't behave badly if a new field is added. Although if you want to actually have the new field show up after using the aggregation/fitting functions you have to add it to the [...]_y_cols mapping in each relevant function.

@mpharrigan mpharrigan added area/otoc Concerns the OTOC module area/ftb_bl Concerns FTb/BL labels May 13, 2022
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Change-Id: Ib0ad42e19989ea513d31fcd406ce075d56c9a6b7
@mpharrigan mpharrigan force-pushed the 2022-04-engineresult branch from 535f9e0 to c34bfb7 Compare August 25, 2022 19:54
@mhucka mhucka enabled auto-merge (squash) September 19, 2025 17:21
@mhucka mhucka disabled auto-merge September 19, 2025 17:21
Copy link
Collaborator

@mhucka mhucka left a comment

Choose a reason for hiding this comment

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

I'm not sure why this was never merged. There are no merge conflicts from updating the branch, and tests pass locally and in CI. I'm going ahead and merging it now.

@mhucka mhucka enabled auto-merge (squash) September 19, 2025 17:24
@mhucka mhucka merged commit 7ec89f4 into quantumlib:master Sep 19, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ftb_bl Concerns FTb/BL area/otoc Concerns the OTOC module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants