zfit / zfit-development

The developement repository for zfit with roadmaps, internal docs etc to clean up the issues
0 stars 2 forks source link

Advanced mode #22

Closed jonas-eschle closed 5 years ago

jonas-eschle commented 5 years ago

@apuignav commented on Fri Oct 26 2018

We need to setup the "advanced mode" that we discussed about. I would suggest something like

zfit.set_lazy_execution(True)

which would set a global variable that one could query with zfit.is_lazy(). This could either be checked by certain functions (minimize, value for FitParameters, things like that? How can we be sure that we do it for everything?) or maybe using a decorator?

@mayou36 What do you think?


@mayou36 commented on Sat Oct 27 2018

Agree!

On the implementation, a decorator would do a great job here. And on the which: every public function possibly returning a TF Graph. I think this is easy to cover every function (and very easy to patch in case we forgot about one).


@apuignav commented on Sun Oct 28 2018

Makes sense, Graphs are only managed internally and that's it. In the end, there's still a great advantage in using TF.

So, if you agree in zfit.is_lazy() I would like to add a module to deal with #34, since most of these functions would only be executed in lazy mode right?


@mayou36 commented on Mon Oct 29 2018

On the name: I am ok with is_lazy(), though it's not my favorite one. I'd may prefer something with graph in it or even the TensorFlow eager (to have a parallel with existing libraries). But lazy is ok as well.

A lot of these high level functions will be lazy executable, but you can use it in both modes, in the end it depends on the flag, so no special treatment needed here, right? But a module which handles all the lazy/non-lazy execution should be there somewhere.

On the flag, I'd propose to keep it (internally for sure) in zfit.settings namespace and probably also make it available in zfit namespace


@apuignav commented on Mon Oct 29 2018

Eager is also good, it's a good idea to keep a parallel with tensorflow. Should we add this to #33?


@apuignav commented on Tue Nov 06 2018

Also, we need a context manager to bundle operations together.


@mayou36 commented on Tue Nov 06 2018

What do you mean exactly? Can you may provide a simple example? Bundle in TF and execute eagerly afterwards?


@apuignav commented on Tue Nov 06 2018

Yeah, that is the idea... Execute a few operations on graphs and then finally do the "eager" execution.


@mayou36 commented on Tue Nov 06 2018

Hm, I see. But let's change it: this contextmanager would only be for advanced users to build things and in generally be hidden, right? So I'd propose a contextmanager with no_eager, so that the global flag will be changed for this time to no_eager and then you have to "manually" run it in a session at the end (or put the last operation outside of the block). Auto-evaluation at the end could be tricky... but going out of the block is easy


@apuignav commented on Tue Nov 06 2018

Yes, sorry. This is exactly what I wanted.


@mayou36 commented on Tue Nov 06 2018

In extend to the advanced mode (which basically implies to provide some kind of environment as in "globals"), there are more use-cases for global flags (like parallelization). I would propose to have a singleton RunManager or similar that handles that. Good name? zfit.manager? (I think instances of a class are in these cases a little bit simpler to handle then modules)


@apuignav commented on Tue Nov 06 2018

In extend to the advanced mode (which basically implies to provide some kind of environment as in "globals"), there are more use-cases for global flags (like parallelization). I would propose to have a singleton RunManager or similar that handles that. Good name? zfit.manager? (I think instances of a class are in these cases a little bit simpler to handle then modules)

Argh, this sounds more and more like ROOT... I really don't like globals. I'm wondering if all the "advanced mode" should be dealt with with context managers and that's it.


@mayou36 commented on Wed Nov 07 2018

I share you're dislike! But to be reasonable:

We may need it at some point to implement an effective parallelization/serialization strategy, but we'll see then.


@apuignav commented on Wed Nov 07 2018

I disagree.

My point is the following: since the code behaves differently according to the flag (return objects are different), the same code cannot be run in normal or advanced mode. As a consequence, do we really want a global flag? Why don't we have eager by default unless inside a context manager? This is good because it only affects advanced users, it's more descriptive and forces advanced users to be explicit. No errors can happen by changing the flag.


@mayou36 commented on Wed Nov 07 2018

Agree! So proposal on the technicalities: we have a global flag for eager and entering the with context changes that to False (and exiting vice-versa). Or do you have another idea in mind?


@apuignav commented on Wed Nov 07 2018

Mmm, do we really need the global flag? I mean, it's a technicality, and I don't mind, but a flag can be changed, and I would prefer if it can't 😉