WIP: Clean geometry - Githubissues

chrlecarpentier commented 6 years ago

Removed dispensable shapes (null length internodes, CutPointBud cylinders and inside sheath) to simplify plants representation

christian34 commented 6 years ago

@chrlecarpentier I have change the geometry to get visible length: can you check the result is Okay for you ? (if you want to test within lpy, may be you can merge #16 in master then in branch before)

pradal commented 6 years ago

@christian34 I have merged the PR #16, do not worry.

christian34 commented 6 years ago

@chrlecarpentier @Emblanc In fact you have the choice between two geometry for the stem:

the one proposed by Christophe (accessible by reverting this branch to commit df87325 ) with one cylinder for the whole stem, and keepiing the topology as in master ( [Bud] Internode [Sheath Blade].
the one I implement, with one cylinder per visible organ (internode or sheath), but with a change of topology of the string : [Bud] Internode Sheath [Blade]. The later is closer to the one I use in adel.

The main difference is for PAR aggregation: with solution 1 you have one PAR value for the whole stem, with solution 2 you have 1 PAR per sheath/internode

Emblanc commented 6 years ago

When I copy this branch on my computer, there are unresolved conflicts. We should probably merge master into this branch or rebase it. We agreed it is best if the person who proposed the PR does it, but I know @chrlecarpentier does not go on GitHub very often. @christian34, you have worked a lot on this PR, do you think you could do it please ?

christian34 commented 6 years ago

Yes sure !

christian34 commented 6 years ago

Master has been merged again: I will add some lines to answer issue #47

Emblanc commented 6 years ago

I have started testing this new branch and there seems to be at least one problem. In this PR, some organs that should be represented are no longer represented. Plants now have "flying ears". mastervscleangeometry

christian34 commented 6 years ago

Yes : the branch is still in progress: I will post a comment when the geometry is...clean again !

christian34 commented 6 years ago

@Emblanc Okay the branch is ready to test ! Using actual primitive area instead of visibility estimate alows to stabilise a bit more the max PAR variation.

Emblanc commented 6 years ago

I tested this branch and it seems to be working fine. However, there is something odd that I do not understand. When I compare the results of a simulation in the clean_geometry branch and in master, here is what I get :

1) Most of the time (for more than 87% of the cases) : sum_primitive > tiller_surface (this is OK for me) 2) Sum_PAR master > Sum_PAR Clean_geometry. This is consistent with 1) since SumPAR = interceptedPAR/surface and interceptedPAR is the same for master and Clean_geometry (very little variation) 3) There are more regressing tillers in master than in Clean_geometry (this is not consistent with 2)). Normally, the more light the plot intercepts, the less tillers regress...

Apart from this behaviour (3)) that I do not understand, I did not see any problem with the branch.

christian34 commented 5 years ago

@Emblanc I remove merge conflict and all test pass: the branch is ready to be merged for me ! (I will base on this one for testing curved leaves)

Emblanc commented 5 years ago

I made a few changes to solve issues #65 #67 #66

openalea / WALTer

WIP: Clean geometry #9