Hacker News Reader

Comment by abainbridge | Hacker News Reader

This code is great fun. Start here to explore the horror: 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.

View on HN

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.

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...

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/

In physics labs everything is wrapped in foil because they use heat tape to bake the shit of it.

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? ;)

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)

  the crash was due to corrupted input data

Bad data shouldn't cause a crash. That's just sloppy.

Indeed: in fact someone must have found the issue eventually, as newer versions exited with an error instead of core dumping.

Sadly, academic constraints (time; results required the day before) prevented me from actually filing a bug report.

Proper input validation is really hard, and a reliable crash on incorrect input is frankly much better than other alternatives such as soldiering on and output trash, or even worse seemingly fine results.

Of course, it sucks if it takes a few hours before you get your crash, that usually tricks you into thinking the software did something wrong and not you. Best is if it dies immediately upon reading the input, and if it does that with an error or a crash isn't crucial, though of course getting a clear error message is always better.

As a comparison, a collaboration internal tool I need to use will print a helpful usage message, and then segfault if you call it without any arguments...

Journals need to start rejecting submissions without code, or with code that doesn't pass peer review by people with a modicum of software engineering skill.

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.

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.

> 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.

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.

Academia is geared towards paper publishing, so why would they bother with code quality? Even if in the long run it would benefit them, its at best a nice to have. Most academia doesn't share code or data anyway.

It's like saying that programmers are professional English writers, so comments and such should be written in perfect English.

IMHO the only valid criticism is if the the simulation modelling is sound and implemented accurately.

At every place I've worked correct spelling and grammar in comments is in fact expected of developers, as is high quality documentation.

(yes I know I've been lucky)

Papers should not be accepted unless they are accompanied by high quality source code.

> IMHO the only valid criticism is if the the simulation modelling is sound and implemented accurately.

I think the best way to tell if it is sound and implemented accurately is to first require that it is implemented as simply (to understand) as it can be. Even then, it is hard to reason about code. But without simplicity, the problem is ten times worse.

I'm tempted to go further and say that source code is formalized thought. I've written a number of simulators in the past, sometimes purely to help me understand a system more fully. There's something beautiful about describing the behaviour of a system in elegant code. When there's nothing left to remove I get much greater confidence that I've fully groked the system. I'd say this is the area where the "computers are like a bicycle for the mind" quote is the most true.

Review by someone claiming to be a ex-Googler here: https://lockdownsceptics.org/code-review-of-fergusons-model/

If the original code was a single file and you’ve ‘fixed bugs’ then what is on GitHub can in NO way be ‘largely what was written’

This code was used to lockdown an entire country - defending the code now based on who it was written by and under what constraints is disingenuous. It must have been known that there were these issues and they ought to have been fixed before using the output for something that had such huge societal impact.

Isn’t this a great example, why epidemiologists shouldn’t write in C++? Aren’t there some Python modules for this kind of stuff?

Edit: Oh this is very old code, that could be an excuse.

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.

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.

Reply on news.ycombinator.com