Untangling - starting with the dust/knu thing#152
Untangling - starting with the dust/knu thing#152allegroLeiden merged 3 commits intolime-rt:masterfrom
Conversation
|
This seems to have some overlap/conflicts with my PR that I submitted 15 minutes later... |
|
Um, yeah - do you have a cunning plan? |
|
It's probably easier to merge your PR and then redo mine on top of that. My PR doesn't have any structural changes apart from those related to the new lineshape/velocity integration stuff (and maybe we'll keep also the old scheme). |
This commit represents a major attempt to untangle some of the convoluted logic of LIME. It includes numerous modifications to structs and the functions that use them, as well as a re-write of the sequence of operations in the main.c:run() function. In detail: - New macro NUM_VEL_COEFFS defined. - Added new elements 'nLineImages' and 'nContImages' to 'configInfo', which are set within parseInput(). These are now used within main() to control which elements of 'grid' are malloc'd and calculated. - Deleted element 'local_cmb' of 'molData'. This is replaced by a local variable which is set within raytrace(). - Deleted elements 'norm', 'norminv' of 'molData'. The purpose of these values seems to have been to avoid under/overflows, but they are unnecessary, and force the molData struct to be allocated and filled even for continuum images. The values 'norm', 'norminv' are no longer calculated or employed. - Added element 'molName' to 'molData', now used to store the name of the molecule read from the moldata file. - New struct 'continuumLine' to contain 'dust', 'knu' values. - Pointer elements (of type double) 'dust' and 'knu' of struct 'populations' have been replaced by pointer element 'cont' of type 'continuumLine'. - A scalar element 'cont' of type 'continuumLine' has been added to struct 'grid'. This allows single values of dust/knu to be calculated for all grid points without allocating memory for the 'mol' element, and also avoids the confusion implicit in using a quantity named for radiating molecular species in a continuum context. - Structs 'pop2' and 'gAux' have been dispensed with. - Deleted function continuumSetup(): this function has basically evaporated as the few things it did have been subsumed into other functions. - The sequence of mallocs and frees of struct grid elements is now much more orderly, tight and logical. - Function calcGridDustOpacity() is now broken into several functions. The dust file is read (if there is one), and the table data are retained and passed to functions that require them to calculate dust and knu values on the fly. - Function done() renamed to printDone(). - Function getVelosplines() renamed calcInterpCoeffs() and re-written. - New function printMessage(). - The interface to planckfunc() has been simplified. - Functions calcLineAmpSpline() and calcLineAmpLinear() have been rewritten with much better self-documentation. - It is no longer necessary to calculate 'gAux' within raytrace(). - Function sourceFunc() has been deleted. It is not used and it is becoming onerous to adapt it to changes in the functions which are used. - The interfaces to sourceFunc_line() and sourceFunc_cont() have been simplified, which has allowed the deletion of their '_raytrace' versions. - The interface to sourceFunc_pol() has been simplified. - Function writeFits() has been moved from main.c to writefits.c.
|
I am going to force push in a minute to try to fix the wrapping of the commit message. Note to self: stop using %$#&^ pico as editor! |
|
Should I in turn have a look at this? We'll also need to think about how we'll eventually merge this and #153 (for example, should we also keep the old velocity integration option in the code). |
Yes please. |
|
I noticed that you read/write dummy variables to keep binary population files compatible with previous versions now that norm/norminv are no longer used. Is compatibility that important? I'm just thinking that if we don't drop the unnecessary fields from binary population files now, we'll probably never will... |
|
I had in mind to break backward-compatibility at release 2.0 (some time in the misty future). Hopefully at that point we would be able to throw away all the horrible CSV I/O altogether. |
|
This looks fine to me. |
|
Should we (well, @allegroLeiden) go ahead with the merge? This is big enough change to make a difference in writing many of the following PRs. |
|
I don't have many hours today before I head off to Lisbon but I'll try to bring this up to date with master. If I succeed then I'll merge it without more ado. |
3 participants
There are a lot of changes in this single commit, but TBH I am about out of patience with nibbling away at major LIME issues over a space of months, having to waste time on copious amounts of repetition of the same mods and merges. I don't understand why some of you are worrying about the exactness of the results produced by this software when in my view it is at least 2 stages away from that point. We know there are problems with the algorithm; but the algorithm fixes are waiting on fixes for the code structure - the kind of thing which should have been sorted out before 1st release. First let's get the structure right, so it is not an enormous pain constantly to have to work around all the knots and peculiarities; then fix the algorithm; then by all means let's look at the numbers. The present PR is designed to make a leap towards fixing stage no. 1, i.e. code structure. There is more that could/should be done but hopefully the present PR accomplishes a major chunk of what was needed. The details of the changes are in the commit.
This addresses issue #151 BTW.