N3FIT dictionaries are confusing

[siranipour]

N3FIT makes extensive use of dictionaries but they very quickly get rather unwieldy, one such example is:

nnpdf/n3fit/src/n3fit/io/reader.py

Line 175 in a70e6f7

def common_data_reader(

which returns a list of dictionaries which map to lists containing more dictionaries which map to more lists... and it just becomes an absolute mess trying to keep track of what it is I am trying to dig to.

There must be an easier way of structuring these things. In general n3fit appeals to dictionaries quite a bit which makes me wonder if there is any other way of doing this.

[scarlehoff]

Yeah, they were supposed to be a temporary thing...

[siranipour]

Did you have any OOP structure you had in mind to replace them?

[scarlehoff]

Yes, my idea was for n3fit to eventually receive just a list of experiments that know what needs to be fitted. Let's call them FittableSpec, they have just a few public methods:

.__call__ (some form of this)
.chi2_function (basically the loss function)
.input_grid()
.some_method_that_deal_with_stats (things like number of data points and what not, this part I'm not sure)
.target_training(k=?)
.target_validation(k=?)
.target_experimental(k=?)

I played with the idea of implementing those three in #797 instead of the function in here but it was very hacky and will introduce unnecessary constraints to #769 and #789 so let's do thing in order.

In my head the chronological order for this would be:

1 - The experiments to be a single layer (this was done in #766)
2 - Everything that's inside io (and in particular reader.py) to disappear (this will happen surely after #769 and/or #789)*
3 - At that point the code can be already cleaned up a lot: I am thinking of things like this function

Once that's done it can be decided to leave anything that has to do with NN inside n3fit, and then FittableSpecs is read by a different version of model_gen that generates the output layers.
On that note, once all the n3fit-badged PR get merged I have a few more in the pipeline that will make model_gen more self-contained.

ps: then there's the NN options that must be read as a dictionary (even if they are a different thing) and the output because that's what hyperopt likes.
But fair enough, the NN options is just a collection of options so no problem having a dictionary there.

*reader.py was marked for destruction from the moment we put the n3fit in the nnpdf repository (it was there just as a wrapper around vp for the external code that was n3fit :P) I just looked and there's even a comment about that in my first PR in this repository I guess it shows how bad of an idea is to say "well, let's merge that, I mean, it is only temporary".

[siranipour]

Thanks for the detailed response! The roadmap seems solid to me. So the pipeline would look something like runcard -> FittableSpecs -> model_gen -> ModelTrainer -> performfit?

I guess there is nothing stopping us thinking about the FittableSpecs now-ish since we'll need to create these before we can think about removing dependence on io/reader.py

[scarlehoff]

No, once the reading is done in python the only thing in reader that will be left is the folds. Given for how long it has been in the pipeline I would wait until the reading is done before planning the next step...

[scarlehoff]

This is now tracked by #1683

[Zaharid]

cc @comane