Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parameter types and bounds #185

Merged
merged 4 commits into from
Jun 24, 2015
Merged

parameter types and bounds #185

merged 4 commits into from
Jun 24, 2015

Conversation

ntessore
Copy link
Contributor

This PR introduces the possibility to give types and bounds to the individual parameters in an object definition. So far, the parameter types only serve to give a number of default bounds on parameters (see below), but they will be more useful in the future.

An example from the Sérsic source is

params
{
    { "x",   POSITION_X },
    { "y",   POSITION_Y },
    { "r",   RADIUS     },
    { "mag", MAGNITUDE  },
    { "n",   PARAMETER, { 0.5f, 8.0f } },
    { "q",   AXIS_RATIO },
    { "pa",  POS_ANGLE  }
};

The second (optional) argument is the parameter type.
There are a number of common, predefined parameter types: POSITION_X, POSITION_Y, RADIUS, MAGNITUDE, AXIS_RATIO, POS_ANGLE.
The generic type for parameters without special, pre-defined meaning is PARAMETER, which is the default.

The third (optional) argument is the definition of hard bounds for the parameter. This serves to limit the possible range of parameter values, for example to exclude unphysical quantities. Lensed will check whether a prior agrees with the bounds, and display a warning if the prior is partially out of the range, or an error if it is fully out of range.

warning: source.n: prior partially outside parameter bounds [0.5, 8]
Part of the prior lies outside of the allowed range of parameter values. Values 
will be drawn from the prior until a valid one is found. If the bounds of prior 
and parameter do not overlap significantly, this can be slow. You might want to 
change the prior accordingly.
error: source.q: prior does not include parameter bounds [0, 1]
The prior does not include the allowed range of values for the parameter. It is 
impossible to draw a valid parameter value. Please fix the prior to include the 
range of values indicated above.

We can also define bounds to be set by default for certain parameter types. So far, these are

  • RADIUS has bounds [0, inf)
  • AXIS_RATIO has bounds [0, 1]

@ntessore
Copy link
Contributor Author

cc @fabiobg83 @rbmetcalf

@ntessore
Copy link
Contributor Author

This needs some fixup to make the tests pass on Travis again before it can be merged.

@ntessore ntessore force-pushed the nt/param-types-and-bounds branch 2 times, most recently from 5b20aeb to feff513 Compare June 24, 2015 09:41
@ntessore ntessore force-pushed the nt/param-types-and-bounds branch from feff513 to 7727c8d Compare June 24, 2015 09:45
@ntessore
Copy link
Contributor Author

This is actually quite hard, as there is inconsistency on what doesn't work when one has strings inside a struct initialiser.
On some OpenCL implementations, it seems that initialising a fixed-size array with a string literal is broken:

char name[16] = "test"; // won't work for some implementations

Unfortunately, on other platforms (e.g. Apple) it appears that assigning a string literal to a constant char* is broken instead:

constant char* str = "test"; // does not work sometimes

The above are just examples to showcase what does not work; in the real case, the strings are members of a struct that is initialised. I'm investigating.

@ntessore
Copy link
Contributor Author

Taking a closer look at what the compiler puts into memory in the first case above reveals the buggy behaviour.

This is what the sie parameter definition should look like, in the format name[16] | type | bounds:

78 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 | 01 00 00 00 | 0.000000 0.000000
79 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 | 02 00 00 00 | 0.000000 0.000000
72 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 | 03 00 00 00 | 0.000000 0.000000
71 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 | 05 00 00 00 | 0.000000 0.000000
70 61 00 00 00 00 00 00 00 00 00 00 00 00 00 00 | 06 00 00 00 | 0.000000 0.000000

And here is what it actually looks like using the offending compiler:

78 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 | 01 00 00 00 | 0.000000 0.000000
79 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 | 02 00 00 00 | 0.000000 0.000000
72 00 03 00 00 00 00 00 00 00 00 00 00 00 00 00 | 03 00 00 00 | 0.000000 0.000000
71 00 05 00 00 00 00 00 00 00 00 00 00 00 00 00 | 05 00 00 00 | 0.000000 0.000000
70 61 00 06 00 00 00 00 00 00 00 00 00 00 00 00 | 00 06 00 00 | 0.000000 0.000000

It seems like the string is not correctly padded to a char[16] array, and the struct alignment is subsequently taken from the first item of the array.

@ntessore ntessore force-pushed the nt/param-types-and-bounds branch from 04f072b to 7727c8d Compare June 24, 2015 13:11
@ntessore
Copy link
Contributor Author

As this is a problem with some OpenCL implementations, I will merge this and open a separate issue.

ntessore pushed a commit that referenced this pull request Jun 24, 2015
@ntessore ntessore merged commit 7d3f09d into master Jun 24, 2015
@ntessore ntessore deleted the nt/param-types-and-bounds branch June 24, 2015 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant