InstituteforDiseaseModeling / covasim

COVID-19 Agent-based Simulator (Covasim): a model for exploring coronavirus dynamics and interventions
https://covasim.org
MIT License
250 stars 223 forks source link

Applied aggregation to the Parameters Module #323

Closed vedarthv closed 2 years ago

vedarthv commented 3 years ago

Applying Aggregate Domain Pattern to parameters.py Module

Introduction

I am a third year computer science student. As a part of our course project, we have to contribute to certain open source projects. In this case, we are asked to contribute by applying an aggregate domain pattern to the project. Hence, we are asked to make two pull requests which correspond to the two places we can apply aggregation.

Abstract

The code in parameters.py was very complex and obscure in its previous state. Mainly, the make_pars method was too large and complex to understand. This is considered to be bad practise in software design, as methods of such size and complexity are very hard to debug. Additionally, there were different types of parameters which were not grouped and classified clearly. So, I have attempted to use an aggregate domain pattern to encapsulate the construction of the different types of parameters into appropriate classes such as Parameters, Vaccine, and Variant.

How were the changes made?

I created three new classes in parameters.py, which are Parameters, Vaccine, and Variant. The construction of the pars object in make_pars is still the same, but the method was modularized by adding helper functions to append the different types of parameters to reduce complexity. Additionally, the vaccine and variant parameters and their functions were aggregated into their respective classes.

Why were the changes made?

Since the previous construction of pars in parameters.py was quite long and packed into one method, we can apply aggregation to solve this problem. Firstly, modularizing make_pars and making it shorter would reduce complexity and increase readability. Additionally, creating a separate class for Parameters, for aggregation purposes, can allow for better testing and ensuring that any invariants are not violated. As, we can track changes to the pars object much better with discrete methods which operate on pars and having a root object which controls access to the different types of parameters. Finally, the vaccine and variant parameters were also grouped into their respective classes Vaccine and Variant to allow for aggregation of the functions which operate the respective type of parameters.

cliffckerr commented 2 years ago

Thanks for the PR -- we're not considering major architectural changes currently, but we appreciate the contribution to open-source software!