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

Make package work on more general numerical types #91

Open
graeme-a-stewart opened this issue Nov 22, 2024 · 7 comments · May be fixed by #97
Open

Make package work on more general numerical types #91

graeme-a-stewart opened this issue Nov 22, 2024 · 7 comments · May be fixed by #97
Labels
enhancement New feature or request
Milestone

Comments

@graeme-a-stewart
Copy link
Member

Betraying its R&D origin (😊) our jet types are currently locked down to Float64. Time to change this! We should have parameterised types T where T <: Real.

To be fair, integer types don't make much sense, but, e.g., ForwardDiff states The target function must be written generically enough to accept numbers of type T<:Real as input.

@graeme-a-stewart graeme-a-stewart added the enhancement New feature or request label Nov 22, 2024
@Moelf
Copy link
Member

Moelf commented Nov 22, 2024

(or just don't target ForwardDiff)

@graeme-a-stewart
Copy link
Member Author

That was more an example of why it might be useful - we certainly want to go to at minimum AbstractFloat to support Float32.

However, I was having a discussion with @lukasheinrich yesterday and he does have interest in differentiable jet finding, so maybe it could really be useful...

@Moelf
Copy link
Member

Moelf commented Nov 22, 2024

that's an interesting idea -- I never thought about the differentiability of anti-kT, what are some possible applications?

I'm thinking about in terms of what is {y} and {x} in the gradient of d{y}/d{x}?

@graeme-a-stewart
Copy link
Member Author

graeme-a-stewart commented Nov 22, 2024

Wouldn't it be more like $\frac{d_{physics}}{dR}$, where $R$ is the cone radius (or $\frac{d}{dp}$, with the clustering power parameter)?

@Moelf
Copy link
Member

Moelf commented Nov 22, 2024

I thought about them but then it doesn't make that much sense to me -- dR is probably scanned / optimized for something, I don't see us pushing a different dR through ATLAS as the new "ak4".

dp is more weird, I guess technically it's a continuous variable, but -2, -1, -0.5, 0, 1, 2 are somewhat special -- also, if we use anything non-integer and non-0.5, running them in HLT will be super slow....

@graeme-a-stewart graeme-a-stewart linked a pull request Nov 26, 2024 that will close this issue
11 tasks
@graeme-a-stewart graeme-a-stewart added this to the 0.5.0 Release milestone Feb 5, 2025
@graeme-a-stewart
Copy link
Member Author

I thought about them but then it doesn't make that much sense to me -- dR is probably scanned / optimized for something, I don't see us pushing a different dR through ATLAS as the new "ak4".

dp is more weird, I guess technically it's a continuous variable, but -2, -1, -0.5, 0, 1, 2 are somewhat special -- also, if we use anything non-integer and non-0.5, running them in HLT will be super slow....

To be fair, yes, the use cases are marginal, but we should certainly be more relaxed than a fixed numeric type - so it's either AbstractFloat or Real - and Real could cover exotic numbers that someone might find useful.

(Sorry, I forgot this thread, but I have been working on this again in #97)

@graeme-a-stewart
Copy link
Member Author

This got stalled a bit in #97 as I cannot work out how to make TiledJet properly parameterised. The problem is that as it natively implements a linked list, it needs to do so being able to mark the beginning/end of the list, which is done with the noTiledJet singleton. I cannot figure out how to create a noTiledJet that is parameterised with the same parameters as the main reconstruction sequence.

I wonder if this could be attacked from another angle, instead of a hand written linked list, use DataStructures.jl's MutableLinkedList.

What do you think @grasph? Was there any obvious optimisation benefit to the current implementation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants