Default parameter duplication and separation of "lookups"

[irm-codebase]

What happened?

This issue is related to #608 and #619, but its solution is slightly different.

At the moment we have default values in three places:

• model.defaults (unused, but declared)
• model._model_data.attrs["defaults"]
• model._model_data["some_parameter"].attrs["default"]

This leads to the need to track this in two places, and possibly users setting stuff in the wrong place and being confused on why nothing is happening. My proposed solution:

• replace model.defaults with a @property that searches in model._model_data with defined defaults. If not there, users should assume that the default value is 0 / nan / None (generally equivalent, at least in terms of math results).
• remove model._model_data.attrs["defaults"], and ensure that the backend and parsers search for stuff in variable attributes instead (assuming None if a default is not set).

Since attributes are saved automatically in netCDF files, this should reduce bloat and code on our side.

Let me know what you think!

Which operating systems have you used?

• macOS
• Windows
• Linux

Version

v0.7

Relevant log output

No response

[irm-codebase]

The other solution is to remove the internal default from the xarray variable (e.g., model._model_data["distance"] no longer shows the default). I'm not a fan of this one, because I see the value of showing attributes closer to the variables/parameters of the model.

The advantage is that calling model.defaults would be faster (you just print the dict), and easier implementation. The first solution requires a filter (model._model_data.filter_by_attrs(default= lambda v: v is not None), and then building a dictionary for the found variables.

I think the first solution is better overall, because in the backend we always know the name of the variable we are processing, so we can access the default directly.

[irm-codebase]

After some thinking, I believe every input parameter SHOULD have a default in model._model_data. If it does not, it means we have an issue in our preprocessing.

[brynpickering]

The reason we have defaults is because there are undefined parameters that still need a default value (e.g. efficiency parameters may not be defined by the user at all but need to be assigned a value of 1 whenever referenced in the math). In the backend, the longer defaults list is used to fill in missing params where necessary. This would be cleaned by having a parameters key in our math, then we could extract default values from there when the user doesn't define data for that parameter.

[irm-codebase]

Heavily in favor of that.
Although we do have defaults for most model.inputs.data_vars (we add them during init).

In the national example the only ones missing this are:

1. 'base_tech'
2. 'carrier_in'
3. 'carrier_out'
4. 'techs_inheritance'
5. 'latitude'
6. 'longitude'
7. 'nodes_inheritance'
8. 'definition_matrix'
9. 'timestep_resolution'
10. 'timestep_weights'

Most of these are obligatory for other things, or calculated during runtime... maybe my "all parameters should have defaults" idea is not good?

[brynpickering]

Yeah, so some are not optimisation problem parameters (they are either lookups - base_tech, carrier_in/out, etc. - or used in preprocessing - lat/lon). I think it's reasonable to set default: NaN if not otherwise given for those parameters that are not lookups.

[irm-codebase]

Agreed. Summary of fix:

• Remove model.default attribute, add as a property as described above.
• We should define which parameters are lookups somewhere. Lookups should have no default, so that if we try to get it for them the code fails fast.
• Add tests to ensure everything else has a default.
• Modify the backend to fetch defaults from self.inputs[param].attrs["default"] instead of self.inputs.attrs["default"].

Sounds good?

[brynpickering]

Modify the backend to fetch defaults from self.inputs[param].attrs["default"] instead of self.inputs.attrs["default"].

Only when updating a parameter, the initial add of parameters needs to call attrs["defaults"] as that is adding empty parameters that don't exist in self.inputs

[brynpickering]

And that also is the issue for the attr -> property suggestion. The attrs["defaults"] dict contains more defaults than those found in self.inputs. If you don't ever give a value for flow_in_eff, it still has a default. This is stored in attrs["defaults"].

[irm-codebase]

Wait... if we want to keep the defaults of unused parameters after init, then the best solution is the opposite:
Just keep them in attrs[defaults]. You do not get the nice print, but you avoid duplication.

[irm-codebase]

@brynpickering After considering #642, probably an even better solution is to have all this stuff in the new CalliopeMath class. It fits it perfectly, I feel.

The only issues will be the lookups. I believe those shold NOT show up in parameters.
Instead, how about clearly defining them them as a global tuple in the parsing file? These will not change often (I think...) and it would lead to better compartmentalization.

[brynpickering]

They will and can change on-the-fly. A user can define any number of inputs that are actually lookups. If we explicitly define parameters in math (#642) then we could just say that anything not defined there is effectively a lookup table.