-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Scene and light units #38
Scene and light units #38
Conversation
When running WALTer with this PR, the values for the intercepted PAR are lower than before (by a 10⁴ factor), which is consistent with a change of units. On the face of it, these changes seem OK to me since we knew our plants used to intercept too much light (by approximatively a 10⁴ factor). But I will need to confirm that these changes of values are indeed correct biologically speaking. This PR passes all the usual tests on my computer. However, when I look at the intercepted PAR, there are very important changes between one day and the next day for some plants (seems like the issue #14 is back). My guess is that the bug is present but that it was not detected by our test (see #40) |
I extend the test to all axes, normalising for incident par : I found mean variation less than 50% for all axes: @Emblanc : is it okay ? We can also refine the test to check min / max variation range |
fix syntax
The new test seems great ! |
I took a closer look at it and it seems that the bug has not been detected because it happens on day 204 - 205 and the test is only conducted on a few days : it is not conducted on these specific days. |
Okay cool that you locate the bug, I will have a look at these specific days ! |
Here is a document showing the results I have when I run the sim_scheme_test. The results are the same as in the test repository. Shift_in_light.pdf |
Okay, I investigate a bit more on what happen on day 205 for tiller (1,2,1) of plant 3. The high PAR values comes from the way sumPAR is computed:
The strange values for sum_PAR occur when the tiller_surface << sum(primitive_area). For example, at the considered time step, the tiller_surface of axe (1,2,1) of plant 3 is less than 1 % of the sum of primitive organ area, whereas for other axes it is from 3 to 50 %. These discrepancies can come from discretisation effects (math cylinder area are not equal to primitive area) and bugs/ problems in visibility computation (it is difficult to assess a priori what part of the primitive is exposed to light when geometric object are intricated and when light comes from several directions) Probably the best solution will be to switch to the representation of 'only visible' part of organs (PR #9) and compute tiller area as the sum of contributing primitives @Emblanc Meanwhile, we can try to limit the bug by limiting the correction to cases where tiller_surface is at least a few % of primitive area (let say 5%) (and then work on PR #9)? Whatever the solution kept, I will have to convert tiller_surface to square meter before dividing organ PAR |
Just note that I updated my previous comment due to a mistake in result aggregation : the problem with tiller_surface is by far less dramatic than what my original comment suggested... |
add helpers for inspection of light results
…4/WALTer into scene_and_light_units
Using correct unit for tiller_surface (cm2 -> m2) in tiller irradiance computation allows to pass the test of maximal variation < 100 for all plants for the whole simulation (the problematic tiller at step 205 disappear, due to proper selection of surviving axis after unit correction). To me the PR can be merged: @Emblanc @pradal is it okay for you ? |
add some columns to debug_PAR_df
I add some lines to automate this check (see check_light_balance in test_light_interception), and found, like you, that total interception is higher than incident PAR on plot_area. This situation may however be perfectly normal :
Generally, we can observe (but some extreme case can exists) that total interception < incoming vertical light on the plot when infinite is True (and this is effectively the case if you run check_light_balance with infinity_CARIBU=1). In this situation, you will also always guarantee that vertical flux through the soil is below incoming light (but to test that, you will have to add primitive representing the soil). |
Thank you for this complete explanation. I'm glad that everything seems normal after all. |
Switch walter to consistent input unit for caribu: output / light responses should be checked before merging
Thris branch was based on PR #17: it may be good to merge that branch first