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

Progress towards coffea 0.7.* update #8

Closed
mat-adamec opened this issue Mar 10, 2021 · 6 comments
Closed

Progress towards coffea 0.7.* update #8

mat-adamec opened this issue Mar 10, 2021 · 6 comments
Assignees

Comments

@mat-adamec
Copy link
Collaborator

mat-adamec commented Mar 10, 2021

I'm in the midst of upgrading to coffea 0.7.* on the coffea-0.7 branch, but given the fact that coffea-casa is still on 0.6.*, I can only test whether it works on futures, which is being grumpy today. Thus, this issue will stay up to track progress and remind me what I still need to do until the upgrade is finished.

So far:

  • Benchmark examples 1-7 should be updated.

But:

  • They need verification (processor on t3 kept dying...)
  • They need confirmation running on Dask.
  • If it works without any issues, I also need to report to the coffea team, because I'm using a hack to be able to run on the public NanoAOD we're using as it has misconfigured cross-references, and Nick doesn't know if this hack will work with Dask.

And in the future:

  • Update benchmark 8 (there's probably a better way of doing it in awkward1)
  • Update template.
  • Update thq analysis.
  • Update thq analysis tutorial.
  • @zorache will need to update topcoffea? I don't know whether it's on 0.6.* or 0.7.*. I can also do this if she has other things to work on. In practice, it's just switching out all the Array.counts and stuff to ak.counts(Array), and some small formatting changes to things like combinatorics.
@mat-adamec mat-adamec self-assigned this Mar 10, 2021
@mat-adamec
Copy link
Collaborator Author

I've verified that examples 1-5 and 7 are working, though they give different results for the cutflow (but the final graph appears to be the same).

Examples 6 and 8 are currently giving different results, and I'm continuing to investigate.

Lastly, Dask is currently not working with coffea 0.7, which impedes testing on Dask.

@mat-adamec
Copy link
Collaborator Author

I've fixed the cutflow issue. ak.count() was behaving differently than I expected, but ak.num() gave the same old results.

@mat-adamec
Copy link
Collaborator Author

The update has been completed. All that remains is a viable run on Dask for all of the examples/* files, which I believe coffea-casa still requires a patch for, and then I will merge the 0.7 branch into the main branch.

@oshadura
Copy link
Member

@mat-adamec can we add a hack from Nick scikit-hep/coffea#468 in each repository?

from dask.distributed import Client

client = Client("tls://localhost:8786")

# Our file is missing some cross-references, so we have to make NanoAOD push warnings instead of erroring out.
# This ultimately isn't a problem, it's just a constraint of the public NanoAOD we're using.
def fix():
    from coffea.nanoevents import NanoAODSchema
    NanoAODSchema.warn_missing_crossrefs = True

client.register_worker_callbacks(fix)

I tested it, it works and in such a way we will not have a regression while testing new images or coffee version.

@oshadura
Copy link
Member

P.S. please also run them on coffea-casa so we can commit them already cached with generated plots :)

@mat-adamec
Copy link
Collaborator Author

The hack has been added and the branch has been merged. The update is officially complete!

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

No branches or pull requests

2 participants