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

fix: Make rochester_lookup compatible with dask_awkward #875

Merged
merged 11 commits into from
Oct 7, 2023

Conversation

jrueb
Copy link
Contributor

@jrueb jrueb commented Aug 3, 2023

Before, rochester_lookup.kSmearMC and rochester_lookup.kSmearMCerror would not work with dask-awkward arrays. This was because of a call to doublecrystalball.ppf (scipy) and numpy.zeros_like.
Fixes #871

@jrueb jrueb changed the title Make rochester_lookup compatible with dask_awkward fix: Make rochester_lookup compatible with dask_awkward Aug 3, 2023
@lgray
Copy link
Collaborator

lgray commented Aug 3, 2023

@ssrothman FYI we got the old version working - we should settle on a plan to correctionlib-ify rochester corrections.

The tool in coffea to extract the current rochester corrections into something pythonic should be a good starting point for onboarding since you got a good bit of the rest of it done iiuc.

@nsmith- @andrzejnovak @NJManganelli - adding you here since we should keep in mind that we'd like to go this direction (correctionlib rochester)

@lgray
Copy link
Collaborator

lgray commented Aug 4, 2023

Let me put the tests in dask and we'll be good to go I think.

@jrueb
Copy link
Contributor Author

jrueb commented Aug 8, 2023

I fixed the test a bit. The issue was that the first argument to ak.unflatten was a numpy array, while the second one was a dask-awkward array. This is not possible with awkward yet.
However, the test now seems to fail because of dask-contrib/dask-awkward#313, which is still left unsolved.

@lgray
Copy link
Collaborator

lgray commented Aug 8, 2023

Thanks I'll take a look. I had to pause on this to take care of other issues.

@lgray
Copy link
Collaborator

lgray commented Aug 16, 2023

@jrueb just checking in here - the underlying issues in awkward / dask-awkward that are impeding progress here are being worked on my Angus and Jim. But it seems we wait on those for now.

@lgray
Copy link
Collaborator

lgray commented Aug 21, 2023

So with some fixes from awkward and dask-awkward this fails in a different way. I'll keep using this to exercise the changes coming down the pipeline that aim to fix the present failure.

@lgray
Copy link
Collaborator

lgray commented Aug 22, 2023

@jrueb FYI scikit-hep/awkward#2652 now fixes the failures in this PR.

@lgray lgray merged commit 5cbe23a into CoffeaTeam:master Oct 7, 2023
5 of 13 checks passed
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.

Rochester corrections not compatible with dask_awkward
2 participants