Conversation
validphys2/src/validphys/config.py
Outdated
| @@ -165,6 +165,13 @@ def parse_pdf(self, name, unpolarized_bc=None): | |||
|
|
|||
| return pdf | |||
|
|
|||
| def parse_load_weights_from_fit(self, name: str): | |||
| """A fit in the results folder, containing at least a valid filter result.""" | |||
There was a problem hiding this comment.
Instead of returning here a fit with a pase, I think it is better to read here the weight paths as a list in a produce rule (that takes load_weights_from_fit as input) and return that.
And if load_weights_from_fit is empty then return None.
This allows other tools (e.g. simunet or colibri) to override the behaviour more easily
There was a problem hiding this comment.
I have changed this to produce a dictionary of paths, each assigned the corresponding replica index to make the extraction of the correct path easier in model_trainer
| f"Fit to load weights from: {load_weights_from_fit} does not contain any weights file in the expected location (replica_*/weights.weights.h5)" | ||
| ) | ||
|
|
||
|
|
There was a problem hiding this comment.
Some of these checks can be in the validphys produce, here you should only need to check whether the load and the load_from_fit are both defined.
For instance, right now this line
fit_folder.glob("nnfit/replica_*/weights.weights.h5"))
is effectively done twice once here once the model trainer. If you put it in the produce instead then it is only once (and with the extras of being able to override it easily and knowing for sure you have a folder, otherwise the check_fit would've failed)
| basis, | ||
| fitbasis, | ||
| positivity_bound, | ||
| load_weights_dict, |
There was a problem hiding this comment.
@scarlehoff I want to allow the option to not define load_weights_from_fit and have load_weights_dict as None but I am not sure how to do this here
There was a problem hiding this comment.
just set =None as default (in the produce, and if you want also here) and then when none do nothing
There was a problem hiding this comment.
When setting the default to None here, it never ran the produce function even when load_weights_from_fit was defined. But I can still just edit the produce function, this will work. I was wondering if there was maybe a way where the produce function doesn't have to be called at all when load_weights_from_fit is None
There was a problem hiding this comment.
I see
produce function doesn't have to be called at all when load_weights_from_fit is None
No, I think if you ask for an action that requires something it runs over all production rules. But it's not a problem.
scarlehoff
left a comment
scarlehoff
left a comment
There was a problem hiding this comment.
Thanks! Seems fine to me. I want to try to do the version for multireplica before merging, I think it should work fine.
n3fit/src/n3fit/checks.py
Outdated
| from validphys.loader import Loader | ||
| from validphys.core import FitSpec | ||
|
|
||
| l = Loader() |
There was a problem hiding this comment.
In the end you are not using these lines right?
| if self.load_weights_dict: | ||
| weights_path= self.load_weights_dict[self.replicas[0]] | ||
| log.info("Loading weights from path: " + str(weights_path)) | ||
| pdf_model.load_weights(weights_path) |
There was a problem hiding this comment.
In principle this should be able to load the weights for all replicas at once, so that it can also be used when running several replicas in parallel. I'll have a go when I test this PR.
validphys2/src/validphys/config.py
Outdated
| @@ -165,6 +165,20 @@ def parse_pdf(self, name, unpolarized_bc=None): | |||
|
|
|||
| return pdf | |||
|
|
|||
| def produce_load_weights_dict(self, fit, load_weights_from_fit=None): | |||
There was a problem hiding this comment.
Why do you need fit here as part of the input?
Co-authored-by: Juan M. Cruz-Martinez <juacrumar@lairen.eu>
ecole41
force-pushed
the
load_weights_from_fit
branch
from
7aabb07 to
82b7975
Compare
|
@scarlehoff Do you understand why the commondata test is failing here? |
2 participants
This PR adds the
load_weights_from_fitoption which allows for the weights from a previous fit to be used as the initial weights of another fit, if the weight files are available for this previous fit.