-
Notifications
You must be signed in to change notification settings - Fork 13
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
zfit loss and minimizer #41
Comments
I'd definitely be interested in seeing how |
Good to know, I'll keep you up to date.
It may performs pretty similar, the advantage is mostly that for improvements, the community works on the same minimizers. And it's more stable, probably better tested etcetc and maintained by zfit (one sorrow less ;) ). Plus the below:
Yes, the good news is you don't need to implement that yourself (or if, then you implement it once for the whole community). The package lauztat does this already and the (emerged from that) upcomming scikit-stats (more stable, better designed) will do that (or be the place to contribute anything like that). It builds on top of the zfit interface, so using zfit for the loss and minimization will allow a seamless transition. |
Hi @mayou36, we've just merged a large refactor of our inference code. It should now be easy to add additional backends such as zfit to flamedisx. |
(sorry for my late reply, it got lost! Don't hesitate to ping me) To end this, either all 10 libraries converge to 1, which usually does not work since they were all built with the specific use-case in mind and "a specific group of people has technical control over it" (so why trust that they don't just tweak it for their case in the future?), or we build a new library and API definition, that is, by design, enough flexible to cover all the use-cases and belongs to the community, anyone can join. And this is zfit. So while what you have looks really nice and is basically the same spirit as what we have in zfit for loss and minimizers, it is more general in zfit, having other use-cases in mind. The best case would be if you could adapt to the loss interface from zfit, then you can move all the minimizers out and just use what is in zfit. Why: this greatly reduces code duplication and maintenance for everyone. Furthermore, it means that we can all work on the same wrapper of e.g. scipy minimizer. (and, since anyone is welcome to join, as soon as something is contributed etc. credits for authorships or papers will of course be given; it's a community project) Technical: I am still working on a full TF2 conversion. If you agree to wrap it to the zfit loss, I will of course help with the conversion, also making a first scetch if you like, and try to make it work, giving you the branch etc. that it should work with etc. What do you think about this? P.S: I know it's a "large request". In my view, it is though the only option to build a maintainable and stable Python ecosystem, guaranteeing that libraries like flamedisx will survive for as long as possible by minimizing any maintenance) |
Hi all, I was wondering what's the status of this, do you have an opinion? |
Hi @mayou36, thanks for your interest! If I understand correctly, your suggestion is to wrap the flamedisx likelihood in a zfit loss, then build any statistics code on top of zfit, or contribute our inference code to existing packages that do so. I think this is a great idea: there's no need for every experiment to develop, debug and maintain its own (non)asymptotic profile likelihood confidence interval setting code. If there was a robust package for this in the python ecosystem, flamedisx could specialize to just computing differential rates for LXe TPCs. We are currently sprinting to a proof-of-concept paper of our method, so it could be a while before we can give this a shot (as you probably figured out from our slow response time!). As a first step, we would probably still do what @pelssers mentioned above, i.e. add zfit as an additional "minimizer" while keeping our existing interfaces, just so we can test everything. Once this works we can integrate things better. |
This! Exactly that's the whole idea behind zfit: providing a stable package with a reliable workflow so that other packages can specialize on specific implementations and this inference part can be shared. Benefiting in code reduction, common bug finding etcetcetc. Because, I fully agree, this stable package is ("was") missing, at least when we started two years ago.
Of course, I understand, I see it as a long-term goal, step-by-step. Just let me know when you get things out and wanna give this a try. |
I've started the |
That looks good, good to see that! I've seen you finished your paper, congrats. Yes, I think it could be good to have an extended discussion, if you are interested, on the whole structure of the project and different design decisions, I am very keen to learn about different choices and the advantages - or disadvantages - of them, see what we may can provide from zfit side, and also to better understand in which direction to develop zfit in order to grasp the generality of model fitting with a clean design. We can schedule by e-mail: Jonas.Eschle@cern.ch |
As already discussed with @pelssers, I would propose you for the long term to make use of zfit, which contains API and workflow definitions for model fitting including implementations. This means that the loss can be simply wrapped by a zfit loss and then the zfit minimizers (they include Minuit) can be used. Furthermore, statistical treatments like confidence interval etc can be performed simply since statistics libraries build on top of the zfit interface.
The idea of this unification is to have a common effort in implementing, wrapping and improving minimizers and not everyone needs to become an expert. Furthermore, it may helps to structure the code more cleanly when separating the loss building and minimization process. In fact, the only task left to
flamedisx
is to build the loss.However... currently, zfit is not fully TF2 eager compatible yet (since all other tf packages for fitting built on the graph mode, congrats to start out with the TF2 ;) the compatibility will come for sure). All that is basically needed though is:
tf.Variable
)This works! We've already tested it at PyHEP, though a few hacks on zfit side (given it was not fully TF2 compatible) were necessary.
What are your thought on this? Do you have any requirements for parameters, minimization or similar that are may not fulfilled or critical?
The text was updated successfully, but these errors were encountered: