GOFAI / glasstone

Python library for modelling nuclear weapons effects
MIT License
83 stars 27 forks source link

Future roadmap #5

Open jmccartin opened 6 years ago

jmccartin commented 6 years ago

Hi, was just curious at all if you were intending on continuing this project at all. Accuracy of models aside, there were a few code things that I'm happy to work on in another merge request if you aren't think of deprecating any of the work here.

GOFAI commented 6 years ago

Sure, I'm definitely open to merge requests. What do you have in mind?

jmccartin commented 6 years ago

So my knowledge of proper python standards as drastically improved since I made my first merge request to the project, where I was just concerned with Python3 compatibility. Now I was curious and so I stuck the thing in an IDE and there were many highlighted errors and warnings 😁 There several things - mostly relating to code tidyness and readability. While I realise this is probably due to the fact that it's adapted from various papers, the project has a lot of dumped-in variables and magic numbers that could probably better long in a configuration file or two.

There's also what appears at first glance to be a lot of code duplication in the soviet parts, and perhaps the whole thing could also be abstracted to have base classes for each type of blast effect, sort of like this:

class BaseEffectModel(object)
|-- class OverpressureModel(BaseEffectModel)
|-- class FalloutModel(BaseEffectModel)
     |-- class WSEG10(FalloutModel)

By doing this you could probably set some class parameters to to control between the soviet and US models, or just have them as inherited classes. Perhaps in the future one could easily add in other fallout or radiation models. One could then create various unit tests for some of the core functions in a proper CI pipeline, but that's another story.

Since this is an area i'm interested in, it could be a nice project to contribute to in the long term, so no goal is too lofty as long as the project is still active.

GOFAI commented 6 years ago

I know the code isn't pretty--this was the first non-trivial project I wrote in Python and I never got around to polishing it like I meant to. I'd definitely welcome code refactoring and cleanup, and especially tests. I'm not sure I see the rationale for putting all the models in the same class hierarchy, though. Part of that is my preference for functional relative to OO-based abstractions, but NWE models are different enough that even those for the same effect often don't share a similar interface or output (WSEG10 and a proper disk-tosser like KDFOC3, for instance). Is a single class heirarchy necessary for the CI pipeline for some reason?

jmccartin commented 6 years ago

No, I just naively thought it might be possible, after further digging you bring up a good point about the differences in interfaces and outputs.

Perhaps I'll make a start on some of the more obvious PEP8, documentation, configuration, and code duplication issues along with adding a few tests and then we can see where to go from there.

With respect to the documentation - overpressure.py seems to be in most need, given the large amount of functions that are full of magic numbers. I think you even mention that in one of the comments. I'm not sure how doable it is (especially as I don't have open access to journals), but it would be good to put page references and perhaps latex markdown in a block string for each function, so one might have a hint of what each function is trying to do.

xznhj8129 commented 6 years ago

I have access to a slack channel where the arms control community of twitter congregates, so if there's anything you need it would be a pleasure to ask around to see if anyone has it handy.

On Jan 10, 2018 6:24 AM, "jmccartin" notifications@github.com wrote:

No, I just naively thought it might be possible, after further digging you bring up a good point about the differences in interfaces and outputs.

Perhaps I'll make a start on some of the more obvious PEP8, documentation, configuration, and code duplication issues along with adding a few tests and then we can see where to go from there.

With respect to the documentation - overpressure.py seems to be in most need, given the large amount of functions that are full of magic numbers. I think you even mention that in one of the comments. I'm not sure how doable it is (especially as I don't have open access to journals), but it would be good to put page references and perhaps latex markdown in a block string for each function, so one might have a hint of what each function is trying to do.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/GOFAI/glasstone/issues/5#issuecomment-356574865, or mute the thread https://github.com/notifications/unsubscribe-auth/AHVEpXMfDa9_t5UXwj3R8Cr08EgWfl-oks5tJJ3dgaJpZM4RWbCf .

GOFAI commented 6 years ago

Sounds good. Here are a few titles that you might ask the arms control community for: Iu. A. Izrael, Radioactive Fallout after Nuclear Explosions and Accidents (Elsevier, 2002). Charles E. Needham, Blast Waves (Springer, 2010).