Feature/data object toml #201

vgro · 2023-04-20T08:10:59Z

This is a list that contains (eventually all) variables that could potentially be stored in the data object. This list should be used to check for consistent variables names and properties across all modules. At the moment, it is just a collection where everyone can add their variables in a central location.

In the future, we might want to split the list according to the source or use of the variables, but this can be discussed when needed. Further, we already spoke about adding a flag for optional variables, but this is also not urgent.

Type of change

New feature (non-breaking change which adds functionality)
Optimization (back-end change that speeds up the code)
Bug fix (non-breaking change which fixes an issue)

Key checklist

Make sure you've run the pre-commit checks: $ pre-commit run -a
All tests pass: $ poetry run pytest

Further checks

Code is commented, particularly in hard-to-understand areas
Tests added that prove fix is effective or that feature works

codecov-commenter · 2023-04-20T08:16:29Z

Codecov Report

Merging #201 (d00cf93) into develop (1659d93) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #201   +/-   ##
========================================
  Coverage    94.22%   94.22%           
========================================
  Files           33       33           
  Lines         1333     1333           
========================================
  Hits          1256     1256           
  Misses          77       77

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

jacobcook1995

LGTM! I added a couple of comments for areas where we might need to settle on a consistent style

jacobcook1995 · 2023-04-20T09:05:45Z

virtual_rainforest/data_variables.toml

+[[variable]]
+name = "bulk_aerodynamic_resistance"
+description = "bulk aerodynamic resistance"
+unit = "s m-1"


We might have to think about how we express this type of unit, I think calling this "m^-1" might be better. Might be somewhat tricky to address now though as it very much depends on a what can be read correctly (which I'm not totally sure on)

Or if it even needs to be parsed. I'm not sure how we'd actually use parsed units unless we attach units to everything.

Good point, I think it is still useful at the moment while we use it as a lookup table, too

jacobcook1995 · 2023-04-20T09:08:58Z

virtual_rainforest/data_variables.toml

+name = "pH"
+description = "Soil pH"
+unit = "pH"
+source = "external + soil"


We probably also need to make sure we can express this consistently. "source1 + source2" definitely would be parseable downstream, but we would need to make sure we use it in every case

oh yes, there was something similar with an abiotic variables, I wanted to put a comment here but forgot. I think it might be best to split in into two variables?

I'm not quite sure what it means when there is external and a model? If it means that soil modifies external pH then it isn't a source, but it could mean that soil provides a default? This could be a list of sources? source = ["external", "soil"]

I wasn't sure how to notate this one, basically pH will be drawn from data but down the line we might introduce a pH model to modify pH as the simulation proceeds (either based on changes in N or due to disturbance scenarios).

This could maybe become "modified_by" rather than "source" as I guess that's what we want to capture here

There is a difference between variables that come from an external source and others that are updated internally. I think they should have different names, such as pH_input or something like that. I used _ref for variables at reference height.

Wouldn't this basically double the number of variables? At least in the soil module pretty much every variable stored in the data object will be initialised using external data, duplicating everything seems like it could get pretty confusing. I guess the problem can be avoided once we store variables over time (rather than replacing everything at each time step) as the initial state will be preserved.

Though maybe I'm confused and you are referring to continuous data from external sources, which the soil module doesn't have any of at the moment (I doubt it ever will)

I was referring to external data that comes as a time series and is used at every time step. I would say initialization is a a different story

Ahhh right I misunderstood then 🙃. Completely agree that we should not be altering continuous input data!

davidorme

LGTM

virtual_rainforest/data_variables.toml

vgro · 2023-04-27T15:04:17Z

I changes the source= to initialised_by= and updated_by= to get around the problem of lists in the source Could you please check that I did your variables correctly?

jacobcook1995

All the soil ones are correct!

vgro · 2023-04-28T08:08:55Z

This is an intermediate solution, however, I merge so that others can add their variables easily. Need to discuss units and use in VR in the future, see #202

vgro added 2 commits April 18, 2023 16:19

variables toml file added

a7bae12

changed names of reference inputs

d00cf93

vgro requested review from davidorme, TaranRallings and jacobcook1995 April 20, 2023 08:11

jacobcook1995 approved these changes Apr 20, 2023

View reviewed changes

vgro mentioned this pull request Apr 21, 2023

List of variables in data object (toml) #202

Closed

davidorme approved these changes Apr 21, 2023

View reviewed changes

virtual_rainforest/data_variables.toml Outdated Show resolved Hide resolved

source changed to initialised_by and updated_by

24f75df

vgro requested a review from jacobcook1995 April 27, 2023 15:04

jacobcook1995 approved these changes Apr 28, 2023

View reviewed changes

vgro merged commit 9e8dc7e into develop Apr 28, 2023

vgro deleted the feature/data_object_toml branch April 28, 2023 08:09

jacobcook1995 mentioned this pull request Jun 8, 2023

Continuous output of the data object #237

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/data object toml #201

Feature/data object toml #201

vgro commented Apr 20, 2023

codecov-commenter commented Apr 20, 2023

jacobcook1995 left a comment

jacobcook1995 Apr 20, 2023

davidorme Apr 21, 2023

vgro Apr 21, 2023

jacobcook1995 Apr 20, 2023

vgro Apr 20, 2023

davidorme Apr 21, 2023

jacobcook1995 Apr 21, 2023 •

edited

Loading

jacobcook1995 Apr 21, 2023

vgro Apr 21, 2023

jacobcook1995 Apr 24, 2023

jacobcook1995 Apr 24, 2023

vgro Apr 24, 2023

jacobcook1995 Apr 24, 2023

davidorme left a comment

vgro commented Apr 27, 2023

jacobcook1995 left a comment

vgro commented Apr 28, 2023

Feature/data object toml #201

Feature/data object toml #201

Conversation

vgro commented Apr 20, 2023

Type of change

Key checklist

Further checks

codecov-commenter commented Apr 20, 2023

Codecov Report

jacobcook1995 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobcook1995 Apr 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidorme left a comment

Choose a reason for hiding this comment

vgro commented Apr 27, 2023

jacobcook1995 left a comment

Choose a reason for hiding this comment

vgro commented Apr 28, 2023

jacobcook1995 Apr 21, 2023 •

edited

Loading