6 ms·

This code is great fun. Start here to explore the horror: https://github.com/mrc-ide/covid-sim/blob/master/src/CovidSim.cpp https://github.com/mrc-ide/covid-sim

This code is great fun. Start here to explore the horror: https://github.com/mrc-ide/covid-sim/blob/master/src/CovidSim.cpp https://github.com/mrc-ide/covid-sim/blob/master/src/CovidSi... I like the way InitModel() crashes (I think) if a global pointer called bmh (short for bitmap header) isn't first initialized by calling InitBMHead() from Bitmap.cpp. I guess it's obvious to academics with giant brains that InitModel() depends on a bitmap existing. But it gets worse - the pointer is declared in CovidSim.cpp but Bitmap.cpp reaches across the module boundary to initialize it. Sweet. Edit: I feel bad now. It's great that this code was made public. I should be encouraging that. I think academics writing code for things like this should probably take an undergrad course on software engineering. I think the one that taught me about Abstract Data Types would be most beneficial the author(s) of this code.

deleted 6y ago
[deleted]
JauntyHatAngle 6y ago
I appreciate you pointing out the flaws, but I fear they sarcasm is not really warranted. Shit code or not, you can call it out without denigrating the guy. It's nice they aired their source code out for people to look at, least we can do is to critique without snark.
- abainbridge 6y ago
  OK, fair point. Snarkiest comment removed. My comments weren't meant to be serious. The problem I pointed out isn't actually a bug, it's just a style of programming that makes the code harder to read/work on than it needs to be.
deleted 6y ago
[deleted]
cataphract 6y ago
I've seen a lot of code coming out of academia that exhibits a similar complete lack of structure and completely ignores the most basic software development best practices (such as having tests). There are exceptions, and research software engineers are a thing, but unsurprisingly most academics focus their careers on the science rather than the code.
- mytailorisrich 6y ago
  Sometimes those people found a startup company, come up with a 'prototype', and are lucky enough to be acquired. Then, you end up having to turn this 'style' of code into a product and to maintain it...
- heisenzombie 6y ago
  It’s the same reason that if you go into a working Physics laboratory, most things will seem to be wrapped in kitchen aluminium foil. There’s better ways of making thermal insulation, EMF shielding, grounding, dust protection... but the foil is /right there/ and the experiment only really needs to work /once/
  - Gibbon1 6y ago
    In physics labs everything is wrapped in foil because they use heat tape to bake the shit of it.
  - SiempreViernes 6y ago
    Hey now, how could anything be better than foil? You can reliably steal enough of it from any nearby kitchen, and it does the job well enough, what's the downside? ;)
- lbeltrame 6y ago
  I once had to debug a crash in a very popular program used in genomics (for those in the know, it was an early version of "samtools" IIRC) and I found it really hard to read. (In the end the crash was due to corrupted input data, so I fixed the data and stopped debugging)
  - masonic 6y ago
    the crash was due to corrupted input data Bad data shouldn't cause a crash. That's just sloppy.
    - lbeltrame 6y ago
spatulon 6y ago
I was part of the GitHub team that helped get the code ready for public release. We fixed a few bugs, reduced memory consumption, made it portable across operating systems, etc., but the code you see is largely what was written by Neil Ferguson and his team. Given your concerns, I would like to mention two things: 1) the code was originally a single source file, so I'm not surprised the module boundaries are imperfect - the code is many years old and the module boundaries have existed for only a few weeks. 2) this was not written by professional software engineers, but epidemiologists, and they were working with limited budget, time, and programming experience.
- mytailorisrich 6y ago
  If the point of the exercise is to allow public scrutiny of models then IMHO the code should not be changed "for public release". The original code as used to produce simulation results that were in turn used for reports/papers/policy should be made public.
  - lbeltrame 6y ago
    > If the point of the exercise is to allow public scrutiny of models then IMHO the code should not be changed "for public release". Some code I've used from scientific papers in the past made a git tag at the time of publication, and said "Use version XXXX and the provided sample data to get the results in the paper", or something of the sort. This could allow review and improvements post-publication.
- thu2111 6y ago
  Epidemiologists are professional software engineers, in the sense that they are paid full time to write code and summarise the output in papers. They might not be "professional" in the sense of the quality of their work but let's not kid ourselves that these people spend half their days in the lab or taking swabs from children in Beijing. Other types of researchers do that - these guys just run sims.
  - mping 6y ago

deleted 6y ago

[deleted]

JauntyHatAngle 6y ago

I appreciate you pointing out the flaws, but I fear they sarcasm is not really warranted. Shit code or not, you can call it out without denigrating the guy. It's nice they aired their source code out for people to look at, least we can do is to critique without snark.

abainbridge 6y ago
OK, fair point. Snarkiest comment removed. My comments weren't meant to be serious. The problem I pointed out isn't actually a bug, it's just a style of programming that makes the code harder to read/work on than it needs to be.

deleted 6y ago

[deleted]

cataphract 6y ago

I've seen a lot of code coming out of academia that exhibits a similar complete lack of structure and completely ignores the most basic software development best practices (such as having tests). There are exceptions, and research software engineers are a thing, but unsurprisingly most academics focus their careers on the science rather than the code.

mytailorisrich 6y ago
Sometimes those people found a startup company, come up with a 'prototype', and are lucky enough to be acquired. Then, you end up having to turn this 'style' of code into a product and to maintain it...
heisenzombie 6y ago
It’s the same reason that if you go into a working Physics laboratory, most things will seem to be wrapped in kitchen aluminium foil. There’s better ways of making thermal insulation, EMF shielding, grounding, dust protection... but the foil is /right there/ and the experiment only really needs to work /once/
- Gibbon1 6y ago
  In physics labs everything is wrapped in foil because they use heat tape to bake the shit of it.
- SiempreViernes 6y ago
  Hey now, how could anything be better than foil? You can reliably steal enough of it from any nearby kitchen, and it does the job well enough, what's the downside? ;)
lbeltrame 6y ago
I once had to debug a crash in a very popular program used in genomics (for those in the know, it was an early version of "samtools" IIRC) and I found it really hard to read. (In the end the crash was due to corrupted input data, so I fixed the data and stopped debugging)
- masonic 6y ago
  the crash was due to corrupted input data Bad data shouldn't cause a crash. That's just sloppy.
  - lbeltrame 6y ago

spatulon 6y ago

I was part of the GitHub team that helped get the code ready for public release. We fixed a few bugs, reduced memory consumption, made it portable across operating systems, etc., but the code you see is largely what was written by Neil Ferguson and his team. Given your concerns, I would like to mention two things: 1) the code was originally a single source file, so I'm not surprised the module boundaries are imperfect - the code is many years old and the module boundaries have existed for only a few weeks. 2) this was not written by professional software engineers, but epidemiologists, and they were working with limited budget, time, and programming experience.

mytailorisrich 6y ago
If the point of the exercise is to allow public scrutiny of models then IMHO the code should not be changed "for public release". The original code as used to produce simulation results that were in turn used for reports/papers/policy should be made public.
- lbeltrame 6y ago
  > If the point of the exercise is to allow public scrutiny of models then IMHO the code should not be changed "for public release". Some code I've used from scientific papers in the past made a git tag at the time of publication, and said "Use version XXXX and the provided sample data to get the results in the paper", or something of the sort. This could allow review and improvements post-publication.
thu2111 6y ago
Epidemiologists are professional software engineers, in the sense that they are paid full time to write code and summarise the output in papers. They might not be "professional" in the sense of the quality of their work but let's not kid ourselves that these people spend half their days in the lab or taking swabs from children in Beijing. Other types of researchers do that - these guys just run sims.
- mping 6y ago