geminiplanetimager / gpi_pipeline

Gemini Planet Imager Data Reduction Pipeline
Other
8 stars 6 forks source link

Add compile_opt calls to all pipeline code #16

Closed dsavransky closed 8 years ago

dsavransky commented 8 years ago

IDL's language defaults are woefully behind the times. This can be improved with compile_opt settings, see http://www.scicoder.org/2011/11/idl-compile-options-you-should-always-use/

I propose we add to all GPI DRP routines the options: "compile_opt defint32, strictarr, logical_predicate"

I talked this over with Patrick and we both agree that not having these settings makes it more likely that you'll waste hours debugging stupid stuff like the fact that nonzero even numbers are by default considered false, different from every other language we use...

dsavransky commented 8 years ago

Recovering rm journals for issue:

dsavransky: Is there still interest in doing this? I have a perl script that could be adapted to adding this line to any prog without an existing compile_opt statement - would take about 30 minutes to put together. Although, here's an interesting question - if you just add this to __start_primitive, would that automatically take care of all of the primitives? The documentation is also ambiguous - does this have to appear in every function, or once per file?

mperrin: For object oriented code, it would need to be added to every single function in the file. Pretty annoying, and potentially not worth doing...

__start_primitive would indeed take care of all the primitives (except for the few lines of code in each that come before __start_primitive). Good call!

I'd say yeah if you can do this in half an hour to cover all the other procedures except the object oriented stuff, then go for it. (But I bet it will take longer than half an hour to debug any issues it turns up with needing to convert ()s to []s for array access.)

dsavransky: Ah. Damn. Forgot about the strict bracket requirement. Okay, putting the automated idea on hold for now. I'll try to add to __start_primitive and see how much it breaks.

dsavransky: Added the compile options to __start_primitive and fixed all incompatibilities. (commit 1831).

dsavransky: commit 1832 makes everything in backbone, drf_guis, and gpitv idl v5 compatible. Didn't add compile_opt to these, but we could probably do it at any time. the utils directory is a nightmare - would probably take a full day to clean these up. Possibly a project for cloudy nights.

ingraham: for version 1.4, I will add the statement (follows) to all primitives. I'll do this as soon as 1.3 is released so there will be 6 months of testing to ensure no errors pop up.

compile_opt defint32, strictarr, logical_predicate

dsavransky: I actually added this to __start_primitive a while back, so there's no reason to add it to every individual primitive.

mperrin: Is it useful to keep this issue open any further or not? I vote that we can close it, since all the primitives are covered.

mperrin commented 8 years ago

Legacy issue migrated from redmine issue tracker. Originally filed by @mperrin on 2012-12-18

No further action needed at this time.