Apply overrides in Land and surfaces #82
Conversation
also change 15 decimal to 14, otherwise errors reported
| @@ -903,7 +903,7 @@ def test_soil_pool(self): | |||
| d1["org-phosphorus"] = disso["P"] | |||
| d1["org-nitrogen"] = disso["N"] | |||
|
|
|||
| self.assertDictAlmostEqual(d1, r1, 15) | |||
| self.assertDictAlmostEqual(d1, r1, 14) | |||
There was a problem hiding this comment.
I don't get it - why did the normal tests change?
There was a problem hiding this comment.
This is the odd one and I'm not very sure what the reason is. After applying the overrides, if I leave it as 15 float accuracy it will report error, but 14 is fine.
There was a problem hiding this comment.
Ohh that number is float accuracy - no problem from me then
wsimod/nodes/land.py
Outdated
| self.surface = overrides.pop("surface", | ||
| self.surface) |
There was a problem hiding this comment.
Is this something that someone would be doing? I wonder if it would interact with data reading?
There was a problem hiding this comment.
Depends on our choices of promoting the 'standard' way I guess, but happy to close this portal if we only want the user to change the surface name via yaml.
There was a problem hiding this comment.
I think remove it probably - otherwise we will need to figure out whether a bunch of stuff (i.e., things around the data_input_dict reading/loading/etc) would need to change. We can make a separate (low priority) issue for this if you think.
wsimod/nodes/land.py
Outdated
| be overridden (keys) and new values (values). Defaults to {}. | ||
| """ | ||
|
|
||
| self.depth /= self.total_porosity # restore the physical depth (root) |
There was a problem hiding this comment.
Maybe depth should be a @property? Possibly is too linked with other things, not sure
There was a problem hiding this comment.
I think we need to try and test. I'll flag this in an issue and try to revise it when I get a bit more time. #95
| for param in overwrite_params: | ||
| setattr(self, param, overrides.pop(param, getattr(self, param))) | ||
|
|
||
| self.subsurface_coefficient = 1 - self.percolation_coefficient |
wsimod/nodes/land.py
Outdated
| self.autumn_sow = True | ||
| else: | ||
| self.autumn_sow = False | ||
| self.harvest_sow_calendar, self.ground_cover_stages, self.crop_cover_stages, self.autumn_sow = self.infer_sow_harvest_calendar() |
There was a problem hiding this comment.
Glad to see some functions!
| (float): readily available water | ||
| """ | ||
| # Calculate parameters based on capacity/wp | ||
| total_available_water = self.field_capacity_m - self.wilting_point_m |
There was a problem hiding this comment.
Should TAW/RAW just be a @property maybe?
wsimod/nodes/nutrient_pool.py
Outdated
| } | ||
| fraction_dry_n_to_fast = 1 - self.fraction_dry_n_to_dissolved_inorganic | ||
|
|
||
| return fraction_manure_to_fast, fraction_residue_to_humus, fraction_dry_n_to_fast |
There was a problem hiding this comment.
Would it make sense to just store these in self inside the function - same as some functions earlier I think. Not sure what the motivation of passing them about is
There was a problem hiding this comment.
Agree - see commit 9641dbf
Co-authored-by: barneydobson <[email protected]>
Co-authored-by: barneydobson <[email protected]>
Co-authored-by: barneydobson <[email protected]>
Apply overrides in
LandandSurfaceand its superclasses.