lime-rt / lime

Line Modeling Engine
http://www.nbi.dk/~brinch/index.php?page=lime
Other
25 stars 25 forks source link

optional moldatfile not so optionnal #14

Closed mwestphal closed 9 years ago

mwestphal commented 9 years ago

@christianbrinch in the cheat sheet we can read that moldatfile is optionnal considering a dust file have been provided.

However not providing a moldatefile fail because par->nSpecies = 0 break the code here

So i assume at least one moldatfile must be provided.

christianbrinch commented 9 years ago

No. If a dust opacity table is given, no moldatfile is needed.You will get a continuum image out.

mwestphal commented 9 years ago

Well it is not how the code is behaving, can you pinpoint the problem ? i do not have the science skills to do that.

mwestphal commented 9 years ago

@christianbrinch i need your help here. At least explain what i should put in the model.c. You have specifically implemented a test and error control here to break if there is no moldatfile : https://github.com/smaret/lime/blob/7bfb3708c61a220b34798a9fca26c5c767c02404/src/aux.c#L124

So I do not understand.

christianbrinch commented 9 years ago

This is in case the user has specified an image cube, that is a multi channel image. In that case , there has to be a moldatfile, otherwise it makes no sense to ask for a line image.

mwestphal commented 9 years ago

ok. I Still do not understand how it is suposed to work. Can you provide me a model.c:input method wich demonstrate this usage ?

Edit : Ok , i begin to understand how it works. Here is my current model.c for continuum setup. http://pastebin.com/dR46yVCC

But it breaks when accessing the abundance in the grid.

No moldatfile -> nSpecies = 0 -> g[i].abun = NULL -> segfault when calling abundance method.

@christianbrinch Is this normal that , in continuum setup nSpecies = 0 ? If so is this normal that in continuum setup there is no abundance in grid ? If so why isn't tere a control of the existence of abundance before accessing them ?

Edit : nSpecies = 0 imply molData is NULL, and continuumSetup cannot work without molData nSpecies > 0 imply moldatfile MUST contain something

mwestphal commented 9 years ago

@christianbrinch Here is a working model.c for continnum setup: http://pastebin.com/HiHjwTN0 You note that dustfile AND at least one moldatfile must be provided, even if the provided file does not exist locally nor on moldat file server.

From a user point a view, using continuum setup correspond to not specifying nchan and velres for the image.

A moldatfile should not be nessecary provided BUT nSpecies must be at least 1, isn't it ? I can fix the code now i think.

(Keep in mind i am sofware engineer, not a chimist or astrophysicist )

christianbrinch commented 9 years ago

Let me try and explain this. You have three options: line image, continuum image, and line image including the continuum.

For the line image, you need to specify a molecule (moldatfile) as well as trans/freq and nchan+velres/nchan+bandwidth/bandwidth+velres. bandwidth=nchan*velres, so you can do any combination of the two parameters. It is optional to specify either transition or frequency. /If/ you choose to also put a dust file in, you automatically get the third option (line image including the continuum). What you get out is a three dimensional image cube, where the number of pixels in the third dimension equals nchan.

For continuum images, you first of all cannot specify a molecule (moldatfile), because not having a moldatfile is how LIME decides to do a continuum image. Secondly, you have to give a dust file. Since a continuum image is two dimensional (only one channel in the third dimension), nchan cannot be set. I don't remember what happens if you set nchan=1. This should be fine, but maybe the code fails anyway, don't remember. Similarly, velres and bandwidth are meaningless for continuum only. You have to specify a frequency, however, since this is how the code knows where to image the continuum. For continuum images, it has to be freq, since transition has no meaning here.

I hope this helps.

mwestphal commented 9 years ago

because not having a moldatfile is how LIME decides to do a continuum image.

I'm sorry, but determining if a this is a continuum only is made here, using img[i].doline wich is positioned here wich is accessed by this test

if((*img)[i].nchan == 0 && (*img)[i].velres<0 ){

consequently, from a user point of view, using a continuum only image means not specifying nchan and vleres in model.c:input. If you set nchan to 1, lime will try a line image.

And not providing a moldata file will result with a nSpecies = 0, wich break completely lime.

I can (and will ) modify the code to works how you describe it, however i need some more information :

  1. In continuum image, nSpecies should be 1 or 0 ? the is an important question because we can see that in continuumSetup, m[0] is used
  2. In continuum image, does writing a population out file is meaningfull (popsout ) ? if so what nmol is suposed to be ?
christianbrinch commented 9 years ago

If you set nchan to 1, lime will try a line image. Right. You can of course have a line image with one channel. nSpecies should be zero for continuum images. In continuum mode you o not write out a .pops file.

smaret commented 9 years ago

My two cents: I think LIME should not be seen as a library with a 100% error proof API. I see it more as a collection of functions that one can use to build his own model (from his/her) model.c file. Granted, there might be some ways to "break" the code if one pick up the wrong parameters combinations. Right now, LIME assumes that the user "knows what he/she does".

Of course this may be a wrong assumption, and ideally one should prevent the user to do stupid things. But this may require a change in API, so that the control flow is easier. For example, we may decide to have a continuum = 1 variable in the parameters, rather than checking the molfile has not been set and that nSpecies is consistent with this.

But changing the API makes users "who knows what they are using" unhappy, because they need to change their model.c. Plus we need to carefully design the API so we don't change it too often (and make users even more unhappy).

So I would suggest to keep on assuming that the set of parameters are consistent for the moment, and fix this once we have decided what the API should be.

@mwestphal: of course if you can implement an control follow easily enough (without requiring an API change), then fell free to do it!

mwestphal commented 9 years ago

@christianbrinch , ok i make sense our the population output file now. Still , I do not understand one thing. In continuum setup function, what is the role of the molData structure ?

@smaret Of course ! We are a long way to a "dumb user" proof library. The present discussion goal is for me to understand how lime continuum setup should work and point out inconstancy in the current code so they can be corrected.

christianbrinch commented 9 years ago

what is the role of the molData structure?

I think that dust opacities and absorption coefficients are stored in molData. This is not elegant, but it would require a major rewrite to introduce a new data structure and carry it around everywhere.

That said, I was thinking about rewriting the entire thing in C++, which would make the whole code more transparent. One thing I would like to get rid of is the linked list grid structure.

mwestphal commented 9 years ago

I agree, the grid structures is very complex and could be simplified, and also use of C++ should always be preferred for project of this size. After working a few weeks on lime, i strongly recommend rewritting it all.

That being said, and since @smaret doest not know well C++ and since my contract about to end, we will have to work with the current code for now.

About molData, i understand how continuum images work and i can use an easy fix concerning the mandatory molData file without breaking it all.

mwestphal commented 9 years ago

Here com the fix that render moldata really optionnal. It still a bit hacked, with a nSpecies forced to 1 without the need of a moldatafile

https://github.com/mwestphal/lime/commit/2255df7f484386a716c431e7982e62959ebf4150