APSIMInitiative / ApsimX

ApsimX is the next generation of APSIM
http://www.apsim.info
Other
134 stars 162 forks source link

AGPRyegrass (and other AGP species?) doesn't seem to be responding to the initial biomass set on the UI #4516

Closed sno036 closed 4 years ago

sno036 commented 4 years ago

@mpandreucci - would you like to see if this is real or not?

mpandreucci commented 4 years ago

Yes, I’ll have a look at it but I do think it’s real because I’ve had another complain about it too.

Get Outlook for iOShttps://aka.ms/o0ukef


From: Val Snow notifications@github.com Sent: Wednesday, December 11, 2019 5:15:58 PM To: APSIMInitiative/ApsimX ApsimX@noreply.github.com Cc: Andreucci, Mariana Mariana.Andreucci@agresearch.co.nz; Mention mention@noreply.github.com Subject: [APSIMInitiative/ApsimX] AGPRyegrass (and other AGP species?) doesn't seem to be responding to the initial biomass set on the UI (#4516)

@mpandreuccihttps://github.com/mpandreucci - would you like to see if this is real or not?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/APSIMInitiative/ApsimX/issues/4516?email_source=notifications&email_token=AKN53J6T2HBKGH6V7ABZRH3QYBSP5A5CNFSM4JZI5GT2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4H7UXTHQ, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AKN53J5GJM7HR6JFHFWZFA3QYBSP5ANCNFSM4JZI5GTQ.

sno036 commented 4 years ago

@hol430 - there is weirdness going on here. @mpandreucci will add more, Varying behaviour going on. Sometimes can put an initial value in and it is used, other times it sticks in the UI but the model doesn't use it, other times it wont stick in the UI, Always it seems that any entered values are overwritten with saved/closed/reopended.

Just us?

InitValuesNotSaving.zip

mpandreucci commented 4 years ago

Ok. @hol353 and @hol430 it looks like there is an instability in setting up the initial biomass DM for the AgPasture species. I used a simulation in AgPasture and the value set as initial DM (different value from the default) works fine for Ryegrass and White Clover. However, if I close the simulation and reopen it, the initial DM value, set at the AGPRyegrass component, goes back to the default value of 1500. Val ran a simulation where she was using AgPasture species (Ryegrass) and the value set as initial DM was not working. So, even though she set that value as 3000, the simulation was still using the default value of 1500. When her simulation is closed and reopened, the initial biomass value (in the AGPRyegrass component) also returns to the default value (just like my simulation did). I know of some other people, who might be running APSIM via Linux, who are having the same problem. Even though they change the initial biomass for AGPRyegrass, APSIM still uses the default value of 1500.

@hol430 could the fact that the initial DM is going back to its default value have anything to do with issue #4494 ?

hol430 commented 4 years ago

This seems to be due to the fact that when we open the .apsimx file, the ModelCollectionFromResource (in this case the PastureSpecies) will read all properties from the model resource and use these property values to overwrite the values stored in the .apsimx file. I think this is the way the system was intended to work - to my knowledge, most released models which are stored as resource don't have properties which you can specify in the UI (?). I think the fix here will be to not overwrite those properties which have a DescriptionAttribute - that is, don't overwrite any properties which you can modify via the property presenter.

However the strangeness doesn't stop here. My understanding of the ModelCollectionFromResource was that we don't serialize (most of) the model itself when we write the .apsimx file, both to speed up de/serialization and to save disk space (and memory). I just had a poke around in the agpasture validation file though and this is not occurring at all. It appears as though the entire model is being serialized when we create the .apsimx file and then deserialized when we open the file again. In fact, it gets deserialized twice when we open the .apsimx file - once when we read the .apsimx file, and again when we replace it with the version of the model stored as a resource. @hol353 what is the intended behaviour here?

mpandreucci commented 4 years ago

I'm very far from understanding all of this so my questions might be very dumb (as it always seems to be the case) and I appologise for them in advance. Does this have anything to do with the fact that AgPasture runs multiple paddocks, making the serialisation needed? Why is PastureSpecies stored as a resource?

hol430 commented 4 years ago

Serialization is basically the process of converting the model into json, which we then write to the .apsimx file. Deserialization is the opposite - it's the process of converting the json into the model objects. So I don't think it's related to the AgPasture stuff being multi-paddock - all models need to be serialized regardless of whether they're in a paddock or not. The problem is that most of the released models (e.g. Wheat) which are stored as resources don't allow the user to input properties like initial DM via the GUI. I'll fix this bug when I get the chance (Monday, probably).

Unsure why PastureSpecies specifically is stored as a resource, @hol353 would know better than me.

hol353 commented 4 years ago

Yes this is complicated. Each species of AgPasture is stored as a resource in the same way as the PMF species are. When the user opens an AgPasture file, the parameter values are copied from these resource files.

I've got a pull request running which should fix the problem reported in this issue. It also fixes #4525.

hol353 commented 4 years ago

This was fixed in PR #4533