Closed brynpickering closed 5 months ago
codecov[bot]
commented
6 months ago Attention: 4 lines in your changes are missing coverage. Please review.
Comparison is base (
33b0672) 95.19% compared to head (e4a7137) 95.65%.:exclamation: Current head e4a7137 differs from pull request most recent head 15f102c. Consider uploading reports for the commit 15f102c to get more accurate results
brynpickering
commented
5 months ago @sjpfenninger for carrier_in and carrier_out as well as to and from, perhaps we can let them work in a special way...
carrier_in/_out:The user could provide it without the carriers dimension as they do in YAML. E.g.:
| techs | carrier_in | carrier_out |
|---|---|---|
| supply_tech | foo | |
| supply_tech | bar | |
| demand_tech | baz | |
| conversion_tech | [foo, bar] | baz |
We would then enforce that when loading the data and process it back into a dictionary to merge into the traditional model definition.
Multiple carriers in a list would need special parsing as they would likely be loaded in as strings ("[foo, bar]") and would need processing back to lists of strings.
to/fromAs with carrier_in/_out, we let users define it as they would in YAML. However, there is the added step that we would need to identify transmission technologies once all data files are loaded and a dummy "traditional" model definition has been created. Then we would go back to the loaded data files and make a check that no parameters were defined over the nodes dimension for transmission technologies. This would then emulate the YAML loading checks, which do not allow a transmission technology to be defined at a node.
The loop back to do the nodes check could be a pain, but manageable I think.
carrier_in and carrier_outcarrier_in/_out are pivoted in model.inputs to have the values as one of the dimensions and the values being binary. carrier_in/_out and would be missing to and from.brynpickering
commented
5 months ago @sjpfenninger the solution I opted for was to allow to/from to be defined in text format in file but to limit carrier_in/out to still be boolean and to raise an error if a transmission technology defines data at nodes in the loaded data from file. This seems to me like a reasonable compromise that stops us having a separate method to load data from file (as in, YAML-esque data that needs to be processed separately to a dictionary).
brynpickering
commented
5 months ago Docs added in #538
Fixes #92
Summary of changes in this pull request:
data_sourcestop-level key to allow loading arbitrary data from file.parentparam ->base_tech.file=/df=functionality.This implementation has required some code in
calliope.preprocess.model_datato align data loaded from file and those from YAML. This would ideally be even cleaner than it is, but it works for now. The approach I'm taking is:base_techandcarrier_in/out- if included in data from file) which goes at the bottom of the tech inheritance chain. I didn't put this in (2) as I don't want it to override some user YAML definition (e.g.,carrier_inis changed by a YAML override compared to what is loaded from file).## REMAINING ISSUES~- Currently, It is very difficult to ensure any amount of YAML definition can be handled. If there is minimal info provided in YAML (e.g., one specific parameter override for one tech) then you have no info available about which techs exist at which nodes except for what you have provided indata_sources. For the national scale example I've set up, inferring which techs are defined at which nodes gets messed up by array broadcasting offlow_cap_max, making the model think that all techs are defined at all nodes.~ EDIT: I think this is fixed. ~~- We probably don't want the national scale example data duplicated in CSV in the calliope module itself. Perhaps we move this to tests? ~~TODO
- [ ] Order of overrides (YAML > data sources or data sources > YAML) and exception behaviour on clashes between data sources and between YAML and data sources (both currently set to silently override) should be configurable.EDIT: leaving "YAML > data sources" order as-is an non-configurable.sheet_nameparam).Reviewer checklist: