flags of carbonicity not working as expected
Closed this issue · 10 comments
sishijing commented
When I set flags. --startingCarbonicity, --priorCarbonicity and --sigmaCarbonicity on the command line, the singlePopMcmc does not run at all. But I can run singlePopMcmc when I set values for these parameters in the base9.yaml file.
tedvh commented
Hi Max -
that makes great sense. Thanks for all this work.
By the way, while you are at it, can you change the default carbonicity from 0.0
to 0.38?
thanks,
-Ted
…On 1/5/17 6:49 AM, Max von Hippel wrote:
What's happening is that the code does not check to see if a value was already
set in the command line args before setting it based on YAML. SO when you
combine YAML with command line args your command line args are superseded but
the YAML args. Since you haven't updated to the latest version, when it fails to
find anything in the YAML, it crashes (see getOrDie in Settings.cpp). If you
update to the latest version, it will not die, but instead ask you to re-enter
the missing value. That said, this behavior is clearly redundant.
What I plan to do is figure out what the defaults are for each of the requested
values and then use ternaries to only update values which have not yet been set.
For example, according to Cluster.hpp, carbonicity is set by default to 0.0. So
in Settings.cpp, I could do something like:
cluster.starting.carbonicity = cluster.starting.carbonicity ==0.0 ? getOrRequest <double>(startingNode,"carbonicity") : cluster.starting.carbonicity;
Instead of
cluster.starting.carbonicity = getOrRequest <double>(startingNode,"carbonicity");
Because I will have to find the default value for each of the values to be
checked, implementing this will be a bit time intensive, so I'll probably wait
to do it until a bit later this week. If possible I hope to also improve the
prompt for variable entree using node name (for example, "please enter your
desired general.verbose.cluster.starting.carbonicity" is much more helpful than
"please enter your desired carbonicity" as there are multiple carbonicities to
be entered).
Hope that makes sense!
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#61 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEOeMhhww7CDb6c1Hq3M3FEMmE2rPltWks5rPJKKgaJpZM4LbOXc>.
maxvonhippel commented
@tedvh should default carbonicity be 0.38 for all carbonicity values (cluster.starting. carbonicity , cluster.priors.means. carbonicity , and cluster.priors.sigmas. carbonicity )? Also, does this mean I should change the default for singlePopMcmc.stepSizes.carbonicity?
tedvh commented
Hi Max -
good question. The defaults should be
0.38 for cluster.starting.carbonicity and
0.38 for cluster.priors.means.carbonicity
yet
0.0 for cluster.priors.sigmas.carbonicity
I don't know about singlePopMcmc.stepSizes.carbonicity . That is a tuning
parameter and should probably be left wherever Elliot put it for now.
…-Ted
On 1/6/17 12:21 AM, Max von Hippel wrote:
@tedvh <https://github.com/tedvh> should default carbonicity be 0.38 for all
carbonicity values (|cluster.starting. carbonicity|, |cluster.priors.means.
carbonicity|, and |cluster.priors.sigmas. carbonicity|)? Also, does this mean I
should change the default for |singlePopMcmc.stepSizes.carbonicity|?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#61 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEOeMtVBYyKAazojYS7UONUyuC6Olq4oks5rPYj0gaJpZM4LbOXc>.
maxvonhippel commented
Sounds good, thanks.
maxvonhippel commented
I take back my comment about logging, I decided the approach I'd taken was too complex so I redid it to just address the particular problem raised here and not try to tackle logging simultaneously. I'll work on that next in another pr.
tedvh commented
Hi Max -
Good eye. Sure, go ahead and make that variable name change. We have changed
the burnin techniques a few times an probably that didn't propagate.
Elliot can also chime in if he recognizes a problem related to this.
…-Ted
On 1/6/17 3:57 AM, Max von Hippel wrote:
@tedvh <https://github.com/tedvh> for some reason the Node
|singlePopMcmc.burnIter| is called |stage2IterMax| in the YAML file. Every
single other variable is named the same thing in the code and the YAML. Can I go
ahead and change it to use |burnIter| in the YAML? (Doing so would make my code
easier to write, and I think it's more logical to use the same thing in both
places anyway.)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#61 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEOeMlMVes59cAgv-i3qrIujMCmRM7bYks5rPbvCgaJpZM4LbOXc>.
tedvh commented
OK, sounds good.
…On 1/6/17 6:44 AM, Max von Hippel wrote:
I take back my comment about logging, I decided the approach I'd taken was too
complex so I redid it to just address the particular problem raised here and not
try to tackle logging simultaneously. I'll work on that next in another pr.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#61 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEOeMtfmqBxICJ0vH4-N0MqUXtq9x_FKks5rPeLRgaJpZM4LbOXc>.
maxvonhippel commented
I deleted my comment about burnin because I found about 4 other variables with name mis-matches. I want to make the names be the same in the YAML, the code, and the command line args, with only a slight difference in that YAML is of form
path.to.node[key] = variable
... and command line args look like:
--pathToNodeKey=variable
... and c++ variables are a bit abstracted, along the lines of:
// sudo code
Node config = whatever;
Node whiteDwarf = some stuff to get whiteDwarf from config;
variable whiteDwarf.someSpecificMetric = getOrRequest(... );Anyway, I plan to go through and document all of the name mismatches and then open a new issue with them. Then you and Elliot can decide what names to use throughout and I will update it accordingly.
maxvonhippel commented
Also @tedvh you can close this issue now.
tedvh commented
sounds like a good plan.
…On 1/6/17 4:04 PM, Max von Hippel wrote:
I deleted my comment about burnin because I found about 4 other variables with
name mis-matches. I want to make the names be the same in the YAML, the code,
and the command line args, with only a slight difference in that YAML is of form
path.to.node[key] = variable
... and command line args look like:
--pathToNodeKey=variable
... and c++ variables are a bit abstracted, along the lines of:
// sudo code
Node config = whatever;
Node whiteDwarf = some stuff to get whiteDwarf from config;
variable whiteDwarf.someSpecificMetric = getOrRequest(... );
Anyway, I plan to go through and document all of the name mismatches and then
open a new issue with them. Then you and Elliot can decide what names to use
throughout and I will update it accordingly.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#61 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEOeMv6FypEJUBEsPDgjglIVV0NJSybEks5rPmX1gaJpZM4LbOXc>.