Conversation
|
Note that This has still to with the |
|
@kndehaan I have pushed a fix for the biogenic output content carriers now |
noracato
left a comment
There was a problem hiding this comment.
I'm worried that the UPDATE part does not have enough validation.
Can you please add tests that show updating nonexistent keys will not work? I have a feeling we will need the DatasetAttributes module after all (it is still on the emissions-gql branch)
| # See Qernel::Dataset#assign_dataset_attributes to understand what's going on: | ||
| call_on_each_qernel_object(:assign_dataset_attributes) | ||
| # Manually assign emissions hash (not a DatasetAttributes object) | ||
| @emissions = dataset&.data&.[](:emissions) || {} |
There was a problem hiding this comment.
Can you explain once more why not to use DatasetAttributes? Like this there is no validation on what users can set as an emissions attribute.
There was a problem hiding this comment.
Good point, I was not really looking at UPDATE yet, so I thought to keep it as light as possible for read.
However, you are right that it's cleaner to just use the DatasetAttributes approach for consistency and validation.
I re-instated the DatasetAttributes approach and added more tests to cover the case of nonexistent keys etc. I expect more spec will be needed based on the changes for 1990 but I think it's best to process that separately.
… and create boutique DE RF module to keep edge cases in one spot
…ular references for Marshall
…co2_output_production_emissions_fossil and spec
…ce of ccs_capture_rate. Update spec
…thod as direct_co2_input_utilisation_fossil is sufficient
…_utilised and emissions_lulucf_removals checks from spec
…now it covers more cases
…ut co2 content approach
* Expand ConfiguredCSVSerializer with node group expansion, molecule support, and emissions CSV endpoints * Add ghg_carrier method to DirectEmissions/MoleculeEmissions and update test fixture of the direct_emissions_csv * Add ghg_carrier to MoleculeEmissions specs and simplify DirectEmissions specs to use faster mock-based approach * Return symbols from ghg_carrier instead of strings
|
Adding myself as reviewer to check the descriptions for the methods (it should be understandable and correct from a modeller's perspective as well). |
| # Total CO2 utilised (consumed as feedstock) at this node. | ||
| # | ||
| # Currently returns only fossil utilisation, as biogenic utilisation is always 0. | ||
| # | ||
| # @return [Float, nil] Total CO2 utilised in kg, or nil if node is not in emissions group | ||
| def direct_co2_input_utilisation | ||
| with_emissions_node do | ||
| direct_co2_input_utilisation_fossil | ||
| # Potentially in the future: + direct_co2_input_utilisation_biogenic (currently 0) | ||
| end | ||
| end |
There was a problem hiding this comment.
I think we decided to remove this method for now, as we currently don't use it right now, right? @louispt1
There was a problem hiding this comment.
Yes this method snuck back in with the csv serialiser merge - I will remove it
Co-authored-by: kndehaan <102598197+kndehaan@users.noreply.github.com>
Summary of changes
Emissions Calculation Framework
ccs_capture_rate) and utilization (viaco2_utilisation_per_mj)Carbon Content Tracing
emissions_skip_crude_oil_mixedge group for forcing weighted mix calculationsGQL & Dataset Integration
Dataset::ImportandEtsource::LoaderSupporting Infrastructure
free_co2_factorfor capture calculations with consistent resultsOther
Note: Atlas reference should be updated once the Atlas
emissionsbranch has been merged.Goes with: