Add better support for custom generators
At the moment it is very difficult to use own or modified random generators in the SRF and CondSRF classes, because we only support using the lookup tables GENERATORS. This PR adds the possibility again to hand over own classes. I added some error checking, ensuring that the class is a subclass of RandMeth. This is not very pythonic, but using duck typing here is pretty tricky. For example, handing over SRF as the generator gives this error only when trying to actually create an srf:
in SRF.__call__(self, pos, seed, point_volumes, mesh_type, post_process, store)
144 name, save = self.get_store_config(store)
145 # update the model/seed in the generator if any changes were made
--> 146 self.generator.update(self.model, seed)
147 # get isometrized positions and the resulting field-shape
148 iso_pos, shape = self.pre_pos(pos, mesh_type)
AttributeError: 'SRF' object has no attribute 'update'
That's why I opted for the strick instance checking. What are your thoughts in this approach?
The failing checks will be successful as soon as PR #248 is merged.
I like this. We should have an abstract base class for generators, since they don't need to be based on the randomization method. In this ABC we could then define all needed methods and properties that should be provided for the SRF classes.
This should be tested.
And one problem I noticed:
The value_type needs to be restricted better then. Since the vector version of RandMeth is not valid for CondSRF, the valid value types should be a class variable (and not a parameter VALUE_TYPES value in the field/base.py module). So I would suggest to add valid_value_types = ["scalar", "vector"] at the beginning of Field and overwrite it in Krige and CondSRF (there it is only need to be explicit, since the underlying kriging class would check the value after setting it in set_generator).
Just added the mentioned enhancements:
- abstract generator class
AGenerator(self-made generators should inherit from that) - checks for subclass of
AGeneratorwhen setting generators -
valid_value_typesclass variable for all Field classes
Thanks for the input! The only thing I don't agree with, is your naming convention of the abstract Generator class. I think I have never seen them with a capital A and I don't think that we need do have extra naming conventions for ABCs. Do you agree? - If yes, I would merge this PR.
Thanks for the input! The only thing I don't agree with, is your naming convention of the abstract Generator class. I think I have never seen them with a capital A and I don't think that we need do have extra naming conventions for ABCs. Do you agree? - If yes, I would merge this PR.
I just copied this naming convention from another project, where I am collaborating and I thought this was a thing. But I agree that Generator reads better and will be more clear for our use cases. I will apply the mentioned fixes from above and merge then.