Fix cases where validator use isn't robust to model changes

[tylerflex]

Validators used to set unspecified fields

• Near2Far.origin
• Near2Far.medium
• Near2Far.currents
• App.app
• Port.mode_indices
• Monitor.colocate
• Planar.length, PolySlab.slab_bounds, PolySlab.set_center, PolySlab.set_length

Validators aren't triggered when all dependencies change (make root validators?)

• @momchil-flex https://github.com/flexcompute/tidy3d/blob/develop/tidy3d/components/data.py#L396 Should this be a "symmetry" validator?
• ModeSpec.bend_axis_given doesn't get called if bend_radius changes.
• TimeMonitor.stop_greater_than_start doesn't get called if start changes.
• validators.validate_mode_objects_symmetry
• validators.assert_unique_names
• validators.assert_objects_in_sim_bounds
• all of the Simulation validators?

[tylerflex]

Default values:

User wants to specify field optionally. If it is left None, we want to use something_else to set its value. Example: Near2Far origin.

Old way:

field: Tuple[float, float, float] = None

@pd.validator('field')
def set_field(cls, val, values):
    if field is not None:
        return field
    return fn(values.get('something_else'))

Issues:

1. If field left as None and then something_else updates at a later time, the value of field will be stale as the validator isn’t called.
2. field is set by the validator upon first call. Then it is not recognized as None, but rather a "hard coded value" and will not be set by something_else if that changes.

Solutions:

1. Make a @property to always return the up to date value of field when called. Use _field in the code internals. If field is None, it stays None in the model.

field: Tuple[float, float, float] = None

@property
def _field(self):
    if self.field is not None:
        return self.field
    return fn(self.something_else)

However, this will require replacing all instances of .field with ._field in the frontend and backend, so we need to be careful.

3. Use a root validator to trigger on any update of the model.

field: Tuple[float, float, float] = None

@pd.root_validator
def set_field(cls, values):
    if values.get('field') is not None:
        return values
    values['field'] = fn(values.get('something_else'))
    return values

However, there is a fundamental problem here in that field will get set to a non-None value upon init and then it will no longer update if something_else is changed. Basically, this erases any knowledge of field being tagged as “optional”.

My opinion

We should do property approach, since the 2nd option means that field is no longer automatically reset by a change in something_else.

[tylerflex]

Stale Validators

Often times, we want to validate a field but the value depends on other fields in the model.

Old way

field: Tuple[float, float, float] = None

# just check if passes validation
@validator
def validate_field(cls, val, values):
    if fn(values.get('something_else')):
         raise SetupError
    return val

# set a new value for val
@validator
def validate_field(cls, val, values):
    return fn(values.get('something_else')):

Issues

• if something_else is changed, this validator doesn’t get called, so we dont know if it passes validation or if the field should be re-set.

Solutions

1. Use a root validator, which gets called at init time and at every change of the model.

field: Tuple[float, float, float] = None

# just check if passes validation
@root_validator
def validate_field(cls, values):
    if fn(values.get('something_else')):
         raise SetupError
    return values

# set a new value for val
@root_validator
def validate_field(cls, values):
    values['field'] = fn(values.get('something_else')):

my opinion

I see no problem with this except:

1. Passing all of the values in root validators could be more costly than just running singular val validation?
2. Working with values dict is cumbersome.

[tylerflex]

Fields dont pass initial type validation

An annoying thing about pydantic is that the validators are run even if the initial type validation fails. In this case, the values[‘field’] of the broken field is just set to None, which can trigger obscure errors.

field: Tuple[float, float, float] = None

# just check if passes validation
@root_validator
def validate_field(cls, values):
    field = values.get('field')
    values['field2'] = field[0]**2 + field[1]**2 + field[2]**2
    return values

If I try to construct

Model(field='blah')

I’ll get one validation error because 'blah' cannot be cast to a Tuple[float, float, float] but also an obscure error in the root validator because it’s trying to find field[0] but field is None.

Old Way

We simply dont handle this, therefore users might get this obscure error if one of their fields is entered incorrectly.

Solutions

Add a pre root validator that will perform a validation between the initial field checking and the rest of the validators. If any of the required items in values are missing, raise a validation error with more friendly message.

field: Tuple[float, float, float] = None

@root_validator(pre=True)
def check_field_not_none(cls, values):
    for field in cls.__fields__:
        if field not in values:
            raise ValidationError(f'field {field} didnt pass initial validation.')

# this wont be run because the above validator failed first
@root_validator
def validate_field(cls, values):
    field = values.get('field')
    values['field2'] = field[0]**2 + field[1]**2 + field[2]**2
    return values

Issues

We’ll still get two validation errors. One for the type check and one for the check_field_not_None. Still potentially confusing.

[momchil-flex]

Fields dont pass initial type validation

An annoying thing about pydantic is that the validators are run even if the initial type validation fails. In this case, the values[‘field’] of the broken field is just set to None, which can trigger obscure errors.

I think you could try @root_validator(skip_on_failure=True)?

[momchil-flex]

Solutions

1. Use a root validator, which gets called at init time and at every change of the model.

field: Tuple[float, float, float] = None

# just check if passes validation
@root_validator
def validate_field(cls, values):
    if fn(values.get('something_else')):
         raise SetupError
    return values

# set a new value for val
@root_validator
def validate_field(cls, values):
    values['field'] = fn(values.get('something_else')):

my opinion

I see no problem with this except:

1. Passing all of the values in root validators could be more costly than just running singular val validation?
2. Working with values dict is cumbersome.

One thing that worries me about this is if all validators will be run when any value is changed. For example I change run_time and the structures_not_close_to_pml validator will get run?

[momchil-flex]

Default values:

User wants to specify field optionally. If it is left None, we want to use something_else to set its value. Example: Near2Far origin.

Old way:

field: Tuple[float, float, float] = None

@pd.validator('field')
def set_field(cls, val, values):
    if field is not None:
        return field
    return fn(values.get('something_else'))

Issues:

1. If field left as None and then something_else updates at a later time, the value of field will be stale as the validator isn’t called.
2. field is set by the validator upon first call. Then it is not recognized as None, but rather a "hard coded value" and will not be set by something_else if that changes.

Solutions:

1. Make a @property to always return the up to date value of field when called. Use _field in the code internals. If field is None, it stays None in the model.

field: Tuple[float, float, float] = None

@property
def _field(self):
    if self.field is not None:
        return self.field
    return fn(self.something_else)

This seems like a twist to the more usual use of properties, where the private value is the stored one and the property is exposed.

    @property
    def foo(self):
        return self._foo

    @foo.setter
    def foo(self, value):
        self._foo = value

I don't know if there's any known issues with that just apart from the fact that it looks a bit weird, but one thing that comes to mind is that the user will have a hard time knowing what the default value is. Instead of being able to check object.field, they would have to know to check object._field or look at the documentation?