NNPDF / nnusf

An open source machine learning framework that provides predictions for all-energy neutrino structure functions.
https://nnpdf.github.io/nnusf/
GNU General Public License v3.0
0 stars 0 forks source link

attempt to update fs #34

Closed RoyStegeman closed 2 years ago

RoyStegeman commented 2 years ago

In the end we probably want to do this outside of the model. For now thisl'll have to do though because otherwise our plotting scripts become useless.

Radonirinaunimi commented 2 years ago

Yes, we should definitely do this. With the version on master, I couldn't load the models (it's been over an hour now).

Is this ready to be looked at?

RoyStegeman commented 2 years ago

Yes I spend yesterday trying this, but it's not feasible. You need a huge number of layers (one for each interpolation) so to do this inside the model is not realistic. Though master should work since the CI was able to produce a report: https://github.com/NNPDF/nnusf/pull/31#issuecomment-1191805590 so that's a bit strange it's not working for you.

The problem with the current master implementation is that the points used for interpolation are evenly spaced in the kinematic grids instead of corresponding to the original datapoints so for example at small-x we may still be missing some resolution. This PR was supposed to fix that but the thing seems to blow up in memory. Nevertheless, the chi2 of master did improve with respect to not doing feature scaling: https://github.com/NNPDF/nnusf/pull/30#issuecomment-1190915763

I think we need to do the scaling before giving the input to the model, but that way we can no longer just save and store the model as we do now. We can probably do a semi-hacky way where we store the model+interpolation function and also load both to produce the plots. Alternatively, since in he end we want to move to LHAPDF grids anyway, maybe we should start thinking about that now.

Radonirinaunimi commented 2 years ago

Yes I spend yesterday trying this, but it's not feasible. You need a huge number of layers (one for each interpolation) so to do this inside the model is not realistic. Though master should work since the CI was able to produce a report: #31 (comment) so that's a bit strange it's not working for you.

It does indeed help given that now after post-fit, 144 replicas out of 150 are kept (as opposed to ~100 before). I think that the problem is that it scales with the number of replicas/models. I did try with 5 replicas/models and (after a while) it is indeed able to generate the report.

Radonirinaunimi commented 2 years ago

Also now the overall fit folder has gone from ~300 Mb to just under a Gb (with the exact same log frequency).

RoyStegeman commented 2 years ago

I think that the problem is that it scales with the number of replicas/models.

Well, if you load all models at once the required memory of course scales linearly with the number of replicas, but the memory per replica should not increase as the number of replicas increases (if that's what you are thinking of).

Also now the overall fit folder has gone from ~300 Mb to just under a Gb (with the exact same log frequency).

Yes I guess the scaling should just be done outside of the model.

Radonirinaunimi commented 2 years ago

Well, if you load all models at once the required memory of course scales linearly with the number of replicas, but the memory per replica should not increase as the number of replicas increases (if that's what you are thinking of).

No, no! Here I meant the time that it takes to load the models (which also scales linearly). But right now, on my computer, it takes about 6 minutes to load a single model, so for 144 replicas it will be 6 * 144 mn. The memory by itself, as far as I could see during the runs, is not a major bottleneck.

RoyStegeman commented 2 years ago

Hmm actually, since when calling the model during he plot production we don't need the more abstract symbolic tensors, maybe we can store the interpolation as part of the model regardless. Just not during training itself, but only for after.

Radonirinaunimi commented 2 years ago

This could be an idea worth exploring indeed. I haven't had a deep look at the FS part yet but I'll do so now.