search

INET-Complexity / housing-model

Agent-based model of the UK housing market.
MIT License
39 stars 26 forks source link

Fixes issues with random number generation #52

Closed davidrpugh closed 6 years ago

davidrpugh commented 6 years ago

@adrian-carro This PR will eventually fix issue #51. So far I have simply created a private instance of a MersenneTwister random number generator in the Model class which I then pass into the constructor of any object in the model that needs a source of randomness.

There are still a few issues in the data/Demographics.java file that I need to address. But I need to understand what this class is doing before making necessary changes.

davidrpugh commented 6 years ago

@adrian-carro I just compared the results of the Results/*/Output-run1.csv files for two different runs. They look identical to me. Is this the correct file to check?

adrian-carro commented 6 years ago

Hi @davidrpugh, yes, the Results/*/Output-run1.csv files are the ones to compare. Thanks for solving this! I'll soon review and merge it.

adrian-carro commented 6 years ago

What are the extra issues in the data/Demographics.java file? I can't see anything not working or needing repair there at this point... but maybe you have already addressed this problems and the initial comment is not any more relevant.

davidrpugh commented 6 years ago

@adrian-carro I fixed the outstanding issues with the data/Demographics.java file.

I looked at the model runs from the code in the master branch and could not find an example of where the output files were different. I haven't exhaustively looked through the files yet. Can you provide an example of a file where the model runs are generating different output for using the same random seed?

It would be ideal to have an example of the bug that we can convert into a test case...

adrian-carro commented 6 years ago

@davidrpugh I detected the problem on a different branch, ML-calibration, and I added a general issue just to remind myself to check this in the near future. It did surprise me as it shouldn't happen and, indeed, I though I had solved this in the past, as checking this was one of the first things I did when I took over the model. So I am tempted to think this was due to some of the (small) changes I introduced within the ML-calibration branch, where some parameter values stop being read from the config file so as to be introduced via command line. In any case, the changes you introduced make very much sense to me and I am happy to keep them. Regarding the problem within the ML-calibration branch, I can have a look at what's causing the problem at some point next week and commit a solution (I really think it must be related to the command line entering of parameter values).

davidrpugh commented 6 years ago

@adrian-carro OK. Sounds good in which case feel free to merge this PR.