Urban-Analytics-Technology-Platform / demoland-engine

A predictive engine for the Alan Turing Institute project DemoLand
MIT License
2 stars 4 forks source link

Engine is not producing deterministic results #1

Closed mkappas closed 1 year ago

mkappas commented 1 year ago

Hi Martin. I've noticed that the indicators are not the same between subsequent runs. Please have a look at the code and the results below:

from demoland_engine import Engine, get_lsoa_baseline

class Environment():
    def __init__(self, lsoa_input):
        self.eng = Engine(lsoa_input)

    def change(self, idx, val):
        self.eng.change((idx[0], idx[1]), val)

def main():
    lsoa_input = get_lsoa_baseline()
    env = Environment(lsoa_input)
    base = env.eng.indicators.copy()

    env.change((100, 2), 0.9)
    print((base - env.eng.indicators).sum())

    env.change((0, 0), 1)
    print((base - env.eng.indicators).sum())

    env.change((1, 1), 0.5)
    print((base - env.eng.indicators).sum())

    env.change((55, 3), 0.9)
    print((base - env.eng.indicators).sum())

    env.change((55, 3), 0.9)
    print((base - env.eng.indicators).sum())

if __name__ == "__main__":
    main()

1st run:

air_quality                 2.403941e-01
house_price                 2.364924e-01
job_accessibility           0.000000e+00
greenspace_accessibility   -5.234117e+06
dtype: float64
air_quality                 2.682240e+01
house_price                 7.055628e-01
job_accessibility           0.000000e+00
greenspace_accessibility   -5.234117e+06
dtype: float64
air_quality                 2.891710e+01
house_price                 5.364033e-01
job_accessibility          -1.852443e+04
greenspace_accessibility   -5.234117e+06
dtype: float64
air_quality                 2.990249e+01
house_price                 3.839983e-01
job_accessibility          -1.852443e+04
greenspace_accessibility   -5.234117e+06
dtype: float64
air_quality                 2.945884e+01
house_price                 3.517221e-01
job_accessibility          -1.852443e+04
greenspace_accessibility   -5.234117e+06
dtype: float64

2nd run

air_quality                 1.000051e+00
house_price                 5.007202e-01
job_accessibility           0.000000e+00
greenspace_accessibility   -5.234117e+06
dtype: float64
air_quality                 2.819492e+01
house_price                 8.542655e-01
job_accessibility           0.000000e+00
greenspace_accessibility   -5.234117e+06
dtype: float64
air_quality                 3.272642e+01
house_price                 8.225269e-01
job_accessibility          -1.919264e+04
greenspace_accessibility   -5.234117e+06
dtype: float64
air_quality                 3.326518e+01
house_price                 7.289391e-01
job_accessibility          -1.919264e+04
greenspace_accessibility   -5.234117e+06
dtype: float64
air_quality                 3.239431e+01
house_price                 6.574579e-01
job_accessibility          -1.919264e+04
greenspace_accessibility   -5.234117e+06
dtype: float64

As you can see, even if we perform exactly the same actions, the results are not deterministic. Also, even within the same run, performing the same action twice (4th and 5th are the same action), back to back, returns slightly different indicators.

I've also printed base and the results are also slightly different between subsequent runs.

Is there any randomness in the engine?

martinfleis commented 1 year ago

Ah yes. I started exposing random_seed but haven't finished. Will fix today.

martinfleis commented 1 year ago

@mkappas I have exposed random_seed and updated models to the recent versions (may still change). You can pass a seed to Engine as shown in the updated docs. Let me know if the issue persists.

mkappas commented 1 year ago

Thanks @martinfleis . Indeed, the results are now fully deterministic.

I've noticed that the code is significantly slower now. I can check again if you wish, to be 100% sure, but is there any chance that the open() command that is now inside the get_indicators() function has anything to do with it?

martinfleis commented 1 year ago

Could be the case. I moved it while I was testing different models as I temporarily added an additional keyword controlling the loaded model. I did not realise it will have an impact on performance. If you can make a PR fixing that I'll merge it. Otherwise I'll have a look at it on Monday.

mkappas commented 1 year ago

Hi Martin. This will drive me crazy. I'm not sure what is happening but putting back the two open() commands outside of get_indicators() didn't fix anything (at least it didn't break something!). I have the commit here but I won't create a PR since the performance is the same. Let me explain with a bit more details what I did:

So, I tested the performance on two different systems, my work laptop and my own personal computer (let's call them work and personal) and I've also tested the code after 3 commits, your "tag v0.1.0" 89694ae, your "tag v0.2.0" 2cb1f45 and my commit (let's call them tag 0.1.0, tag 0.2.0 and makis'). For the following, I've also used conda environments with Python 3.10 and I'm reporting seconds per step.

work personal
tag 0.1.0 3-4 0.38-0.41
tag 0.2.0 6-7 0.42-0.43
makis' 6-7 0.42-0.44

We can see two things here:

  1. For some reason, the work laptop is performing very poorly (while it's quite powerful I would say).
  2. Between versions 0.1.0 and 0.2.0 something changed that affects the performance, at least on the work laptop.

I'm not sure if we want to put more effort into this right now, unless you know what is happening, since in the next couple of weeks we will test everything on Baskerville as well.

martinfleis commented 1 year ago

huh, no idea what could be causing such a huge difference between your work and your personal machines. we may need to do some profiling to figure that out.

martinfleis commented 1 year ago

That change in get_indicators is not expected to make any difference as it is the function used for our modelling, not within the engine.

That minor difference you see on your personal laptop could be caused by different models the new version has, but that is negligible. The difference on your work laptop I still don't understand.

martinfleis commented 1 year ago

This has been already resolved some time ago.