Skip to content

Inpars#77

Merged
smaret merged 10 commits intolime-rt:masterfrom
allegroLeiden:inpars
Jul 13, 2016
Merged

Inpars#77
smaret merged 10 commits intolime-rt:masterfrom
allegroLeiden:inpars

Conversation

@allegroLeiden
Copy link
Copy Markdown
Contributor

The intent of this is to separate the previous single inpars struct into two: one which contains ONLY those parameters the user can set, and the other for general program information (which may include some of the user pars). Besides using different variables to express different functionality, this makes it easier to change names for example without affecting the user interface. I had to make minor changes to code in a lot of places, but the result (i) compiles, (ii) passes valgrind tests, and (iii) produces an identical image to the previous.

The new struct is typedefd as configInfo. It contains all the information which was
previously stored in inputPars: the difference is that inputPars has now been trimmed to
contain _only_ those parameters which are settable by the user in the model.c file. Any
non-user-settable values should now be added to the config struct.

The advantages of this are as follows.

	- We will usually want more config parameters than user-settable ones. The separation
          leaves it clearer which things the user needs to (or can) set.

	- The separation makes it easier to check for and exclude impossible combinations of
          parameters.

	- We can adopt new names (for clarity) for config parameters without bothering the
          user with a changed interface.

Two small unrelated additional changes:

	- A new macro MAX_NIMAGES is defined in lime.h and used in the initial malloc of struct image
          instead of MAX_NSPECIES as before.

	- Fixed the order of functions in lime.h so they are all (instead of just mostly, as before)
          alphabetical.
This input parameter had no present function in the code, so was removed from there in the previous commit.
Now I also removed mention of it from the user manual.
@allegroLeiden allegroLeiden mentioned this pull request May 23, 2016
@smaret smaret added this to the Release 1.6 milestone Jun 13, 2016
@allegroLeiden
Copy link
Copy Markdown
Contributor Author

I guess you mean ifndef? Good idea though.

@rschaaf-aifa
Copy link
Copy Markdown
Contributor

O yes:

#ifndef LIME_H
#define LIME_H
.....
#endif /* LIME_H */

Following advice from @rschaaf I have created a separate header file inpars.h just to define
the struct for the user-input parameters.

I also included macros in both lime.h and inpars.h to prevent multiple inclusion of these
files.

The final change was to the interface of openSocket(). This had the inpars struct as one of
its arguments, the other being the species ID, but it turns out that all it needs is the
name of the moldatfile for that species. Thus I replaced the former two arguements by a
single char * moldatfile.
@allegroLeiden
Copy link
Copy Markdown
Contributor Author

I just force-pushed a commit which includes the macro test you proposed @rschaaf-aifa but undoes the move of the function prototypes to inpars.h. Hope I have not mangled the commit log too much.

@smaret smaret self-assigned this Jun 30, 2016
@allegroLeiden allegroLeiden mentioned this pull request Jul 7, 2016
This was a pretty difficult merge. In my hacks I tried to preserve the principle that ONLY the user-settable inpars should be passed
between functions at the top level inside main(), i.e. that the new configInfo stuff should be sequestered inside run().
@smaret
Copy link
Copy Markdown
Contributor

smaret commented Jul 12, 2016

I've just merged your PR #105 and now this branch has conflicts again... Sorry about this.

Let's us first merge this one so you don't have to fix conflicts in your other branches over and over again.

@allegroLeiden
Copy link
Copy Markdown
Contributor Author

Serial conflict fixing seems to be unavoidable when the PRs have built up like this. I think we should try to be a bit more sequential with the next release.

@smaret smaret merged commit 981039b into lime-rt:master Jul 13, 2016
@allegroLeiden allegroLeiden deleted the inpars branch September 7, 2016 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants