[iebaltab] March 2022 rewrite review

[bbdaniels]

General:

• Rename main branch
• run-old-versions.md link in deprecation error message 404s

Help file / Options:

• Output options descriptions need to be updated (save, browse)
• The meaning of grpcodes is unclear -- I cannot determine what it does
• "Statistics" options should be listed right after the mandatory options, then, separately, the "data treatment" ones. These are much more important than labelling!
• "F-test" options can be added to the statistics options. What are missing values in F tests? fnoobs should be with the display options.
• What does missminmean() do?
• pairoutput option accepts multiple inputs, such as pairoutput(beta t), which I want to do, but then causes error and returns: option pout_lbl() required without explanation. Recommend to also allow se and ci.

Math:

• Default set of comparison tests (all vs all) is good
• ⚠️ I cannot quite understand what ftest is doing -- is this the omnibus test? If so I would enable it by default.
• ⚠️ SEs and SDs for groups do not display
• pairoutput option could be required, which would remove any issue of default functionality? It could also be called something more intuitive like Stats()
• ⚠️ I am not sure that the nrmd (normalized differences) are correctly calculated. See this link:

The normalized differences approach of Imbens and Rubin (2015) is useful here. This is defined as the difference in means between the treatment and control groups, divided by the square root of half the sum of the treatment and control group variances

• control() option should flip all regression signs (and subtraction order)
• New treatment() option should replace current control() option functionality
• There is no need for a separate fixedeffect() option as long as indicator i. variables can be appropriately included in the covariates() option
• What does covarmissok do? Could this potentially be combined with the requested casewise option somehow, since it appears conceptually similar?
• I would remove the balmiss(), balmissreg(), covmiss(), and covmissreg() options, or at least move them somewhere else ("Advanced"?)... this is quite dangerous....

Display:

• rowvarlabels behavior should be the default, with the option to disable
• Table notes should explain the meaning of non-numerical results like .v.
• The default paired t-tests should also report their SEs by default, with the option of instead producing P-values and, possibly, confidence intervals. These options need to be updated in the help file in any case:

The options nottest, normdiff, pttest, pftest and pboth have been deprecated as of version 7 of iebaltab. See if the
    options pairoutput or ftestoutput have the functionality you need. If you still need the old version of iebaltab,
    for example for reproduction of old code, see this guide for how to run old versions of any command in the ietoolkit
    package.

Misc:

• onerow returns an unhelpful error if the number of observations varies across regressions. The follow, for example, occurs, but the number of obs in the regressions is not reported in the table without the command, so the source of the failure can't be found.

iebaltab price mpg rep78 , grpv(rep78) savexlsx("test.xlsx") replace control(2) total grpc onerow

Option onerow may only be used if the number of observations with non-missing values are the same in all groups
    across all balance variables. This is not true for group(s): [2_1 2_3 2_4 2_5]. Run the command again without this
    options to see in which column the number of observations is not the same for all rows.

[kbjarkefur]

SE and SD went missing when I capitalized the titles. It is fixed in 33117f7

[kbjarkefur]

Yes, ftest does that test. This is still accurate in the old helpfile:

reg testgroupdummy balancevarlist
testparm balancevarlist

I prefer to keep it an option for backward compatibility reasons and that it is cleaner IMO to add options for things you add instead of add options to remove things. I can also see someone who does not like f-test come with feedback saying that we should not promote this or that test.

[kbjarkefur]

I like stats(). And then it would be specified like stats(pair(t) f(p))

[kbjarkefur]

The options missminmean(), covarmissok, balmiss(), balmissreg(), covmiss(), and covmissreg() are among the options I said will be deprecated. They are soon gone so dont worry about them 😃

[bbdaniels]

I like stats(). And then it would be specified like stats(pair(t) f(p))

I don't think it even needs the sub-functions -- if it always takes two arguments then it must be some combination of the specification for the estimate and the specification for the uncertainty. ie, stats(mean se) or stats(beta ci) is completely unambiguous

[kbjarkefur]

I don't think it even needs the sub-functions -- if it always takes two arguments then it must be some combination of the specification for the estimate and the specification for the uncertainty. ie, stats(mean se) or stats(beta ci) is completely unambiguous

Preference can and are often different across tests and, for example, ci does not work on an F test. It doesn't make sense to me to replace pairoutput with stats() and then keep ftestoutput() and feqtestoutput().

The syntax will be stats(pair(string) f(string) feq(string)) where those three strings allows different input.

I can see group() being added in a future version.

[kbjarkefur]

"F-test" options can be added to the statistics options.

No due to backward compatibility

What are missing values in F tests?

I assume that this is comment in relation to fmissok that is now a deprecated option

noobs should be with the display options.

done in 11d283b

[kbjarkefur]

Default set of comparison tests (all vs all) is good

I do not understand what this means

[kbjarkefur]

pairoutput option accepts multiple inputs, such as pairoutput(beta t)

We decided in the meeting that this is not part of this version

causes error and returns: option pout_lbl() required without explanation

This option is replaced by stats()

Recommend to also allow se and ci.

Done for stats(pair(se)) and stats(pair(sd)) in f9b2a26

ci has two values which require bigger changes. Possible to implement but out of scope in the re-write.

[kbjarkefur]

pairoutput option could be required, which would remove any issue of default functionality?

No due to backward compatibility

It could also be called something more intuitive like Stats()

Done in d3705ba

[kbjarkefur]

I am not sure that the nrmd (normalized differences) are correctly calculated. See this link:

@bbdaniels, can you give me a correct formula? Preferable in terms of Stata code? I understand a text like this, but I need your help to translate this into code at the level of accuracy a big command like this requires

[kbjarkefur]

New treatment() option should replace current control() option functionality

Hesitant to remove control() due to backward compatibility, but if you elaborate on the benefit of a new option treatment() is and why it must replace control() I am happy to consider it

[kbjarkefur]

There is no need for a separate fixedeffect() option as long as indicator i. variables can be appropriately included in the covariates() option

I don't disagree if the command was new. I do not see an issue with fixedeffect() big enough to break backward compatibility. Let me know if there is anything I am missing.

[kbjarkefur]

rowvarlabels behavior should be the default, with the option to disable

Again, I don't disagree if the command was new, but do not see what would justify to break backward compatibility. Let me know if there is anything I am missing.

[kbjarkefur]

Table notes should explain the meaning of non-numerical results like .v

Are you sure the table notes is the right place for this? Missing values should never end up in the table. And I see a big risk for confusion if the table note explains things that are found in other places rather then the table.

I was thinking of explaining this in the helpfile. Only people skilled enough to look in the helpfile would be skilled enough to use return list to access a martix.

[kbjarkefur]

The default paired t-tests should also report their SEs by default, with the option of instead producing P-values and, possibly, confidence intervals.

Risking to sound like a record on repeat, I think backward compatibility is important so I am reluctant to change default behavior.

Multiple stats for the pair tests is out of the scope for this re-write.

These options need to be updated in the help file in any case:

The first section of the helpfile is updated in 3a0a9f8 The rest of the helpfile will be updated once we have finalized the code of the command

[kbjarkefur]

onerow returns an unhelpful error if the number of observations varies across regressions.

Agreed, I will implement

[bbdaniels]

There is no need for a separate fixedeffect() option as long as indicator i. variables can be appropriately included in the covariates() option

I don't disagree if the command was new. I do not see an issue with fixedeffect() big enough to break backward compatibility. Let me know if there is anything I am missing.

I don't think it needs to break anything -- just deprecate it and remove the option from the help file on this version. It will still work backwards, but people should prefer to use the modern syntax directly as a covariate -- for example, things like two-way fixed effects might require explicit regression specs. I don't think you need to do any more coding to make that work correctly since it just passes through the string, but I will test and check.

[bbdaniels]

New treatment() option should replace current control() option functionality

Hesitant to remove control() due to backward compatibility, but if you elaborate on the benefit of a new option treatment() is and why it must replace control() I am happy to consider it

I think this would have no impact other than flipping the signs, which I understand is not ideal, but it would align better with the normal meaning of "treatment" and "control". So treatment() would do what control() does now, which is present the treatment effect for one group relative to all other groups. And control() would do the opposite, which is present treatment effects for all groups relative to the one specified.

Alternatively, to avoid breaking, control() could be deprecated and hidden -- that is, just pass through the treatment argument to the existing functionality so as not to make maintenance harder, and use a new term like base() for the option that flips the comparison. I would think that would not be terribly difficult and would preserve both backwards compatibility and maintainability?

[bbdaniels]

Default set of comparison tests (all vs all) is good

I do not understand what this means

I just mean this is a good default and I like that you have done it. No need for any changes ☺️

[bbdaniels]

"F-test" options can be added to the statistics options.

No due to backward compatibility

What are missing values in F tests?

I assume that this is comment in relation to fmissok that is now a deprecated option

noobs should be with the display options.

done in 11d283b

Sorry -- the first comment was only to refer to the help-file organization, not the functionality. I will suggest improvements there in commits on my next run-through.

[bbdaniels]

I am not sure that the nrmd (normalized differences) are correctly calculated. See this link:

@bbdaniels, can you give me a correct formula? Preferable in terms of Stata code? I understand a text like this, but I need your help to translate this into code at the level of accuracy a big command like this requires

Yes, I will do this. Edit: Done. But needs resolution on comment below about which difference will be used. Should be the adjusted difference!

The correct formula will be:

abs(`pair_difference') / sqrt((`variance_1'+`variance_2')/2)

[bbdaniels]

Table notes should explain the meaning of non-numerical results like .v

Are you sure the table notes is the right place for this? Missing values should never end up in the table. And I see a big risk for confusion if the table note explains things that are found in other places rather then the table.

I was thinking of explaining this in the helpfile. Only people skilled enough to look in the helpfile would be skilled enough to use return list to access a martix.

Hmm, I'm not sure why I was getting them then. I'll test again and see what I'd done and let you know.

[bbdaniels]

One more big thing that I somehow missed before -- is it correct that the output table does not reflect the actual regression coefficients when the covariates() option is used?? I think is absolutely essential to get the adjusted coefs here, and IMO this should by default -- as with the other options, I am happy to suggest we deprecate and remove the covariates() option so it works with past code but is not noted for new users (and, should return a warning). I have used, for example, rhs() for such an option.

[bbdaniels]

Ok I am trying to test the stats() stuff but I can't find direct instructions. When I tried normd as in the help file I get:

The options nottest, normdiff, pttest, pftest and pboth have been deprecated as of version 7 of iebaltab. See if the options pairoutput or ftestoutput have the functionality you need. If you still need the old version of iebaltab, for example for reproduction of old code, see this guide for how to run old versions of any command in the ietoolkit package.

But there is no pairoutput since it is stats() now, right? The stats_string definitions don't seem to be in the help file yet so I can't test thoroughly, but I can back it out from the parser and use:

iebaltab trunk weight length , grpvar(rep78) savex("/users/bbdaniels/desktop/test.xlsx") covar(price) replace stats(pair(nrmd))

Then I get normalized differences for all comparisons. For these, we will replace by the formula above. I don't think these use significance stars in any case so those will drop as well -- the magnitude should be the only metric reported.

[kbjarkefur]

There is no need for a separate fixedeffect() option as long as indicator i. variables can be appropriately included in the covariates() option

I don't disagree if the command was new. I do not see an issue with fixedeffect() big enough to break backward compatibility. Let me know if there is anything I am missing.

I don't think it needs to break anything -- just deprecate it and remove the option from the help file on this version. It will still work backwards, but people should prefer to use the modern syntax directly as a covariate -- for example, things like two-way fixed effects might require explicit regression specs. I don't think you need to do any more coding to make that work correctly since it just passes through the string, but I will test and check.

The two tables blow was created using syntax iebaltab weight price , grpvar(tmt_cl) [...] cov(mpg) fixedeffect(foreign) stats(pair(beta)) and iebaltab weight price , grpvar(tmt_cl) [...] cov(mpg i.foreign) stats(pair(beta)) and are identical. So the functionality you are suggesting exists (although by luck). Please let me know if you see a reason to still remove fixedeffect() that justify it breaking backward compatibilty.

[kbjarkefur]

New treatment() option should replace current control() option functionality

Hesitant to remove control() due to backward compatibility, but if you elaborate on the benefit of a new option treatment() is and why it must replace control() I am happy to consider it

I think this would have no impact other than flipping the signs, which I understand is not ideal, but it would align better with the normal meaning of "treatment" and "control". So treatment() would do what control() does now, which is present the treatment effect for one group relative to all other groups. And control() would do the opposite, which is present treatment effects for all groups relative to the one specified.

Alternatively, to avoid breaking, control() could be deprecated and hidden -- that is, just pass through the treatment argument to the existing functionality so as not to make maintenance harder, and use a new term like base() for the option that flips the comparison. I would think that would not be terribly difficult and would preserve both backwards compatibility and maintainability?

I agree with your high level point and is softening up to this. However, where I have yet to understand what you envision is in the details.

If iebaltab were new I think control() would be the best option and it would take the integer of the value in the variable in grpvar() that is control. And then the signs would be flipped so a positive value in the pairwise test column equals a positive effect in that treatment vs. control.

In a multiarm treatment situation, what do you envision treatment() to take as values? Let's say grpvar() has values 0, 1, and 2, where 0 is the control group. Should the user then specify treatment(1 2) in your vision?

[kbjarkefur]

I am not sure that the nrmd (normalized differences) are correctly calculated. See this link:

@bbdaniels, can you give me a correct formula? Preferable in terms of Stata code? I understand a text like this, but I need your help to translate this into code at the level of accuracy a big command like this requires

Yes, I will do this. Edit: Done. But needs resolution on comment below about which difference will be used. Should be the adjusted difference!

The correct formula will be:

abs(`pair_difference') / sqrt((`variance_1'+`variance_2')/2)

Where `pair_difference' I know where to get. But where, expressed in Stata specifics, would you get `variance_1' and `variance_2'? Can you point to something in return list or ereturn list following regress I can use for this?

[kbjarkefur]

Table notes should explain the meaning of non-numerical results like .v

Are you sure the table notes is the right place for this? Missing values should never end up in the table. And I see a big risk for confusion if the table note explains things that are found in other places rather then the table.
I was thinking of explaining this in the helpfile. Only people skilled enough to look in the helpfile would be skilled enough to use return list to access a martix.

Hmm, I'm not sure why I was getting them then. I'll test again and see what I'd done and let you know.

You are expected to get them in r(iebaltabrmat) and r(iebaltabfmat), but please send any reproducible example where they end up in the table my way. That would be interesting to look into.

[kbjarkefur]

One more big thing that I somehow missed before -- is it correct that the output table does not reflect the actual regression coefficients when the covariates() option is used?? I think is absolutely essential to get the adjusted coefs here, and IMO this should by default -- as with the other options, I am happy to suggest we deprecate and remove the covariates() option so it works with past code but is not noted for new users (and, should return a warning). I have used, for example, rhs() for such an option.

It does not affect the value displayed in the output if stats(pair(diff)) is used (which is the default) as covariates does not change the real difference between the two groups on the ground. And the most basic usage of a balance table is to describe the difference between the two groups and test if the difference is significant. And for the most basic usage stats(pair(diff)) makes it straightforward to say what the number in the table is.

If you use covariates and stats(pair(diff)) then the test if the treatment statistically significant explains the difference in means has changed, however, the difference between the groups has not changed in reality and not in the table.

When you are using no covariates or fixed effects then there is no difference between stats(pair(diff)) and stats(pair(beta)). But when you do use covariates or fixed effects there is a difference. But it becomes confusing for a layman to know what the value in the table means if it is the beta coefficient that is displayed.

Both are used, and stats(pair(beta)) might be more appropriate for an academic paper, but since the meaning of stats(pair(diff)) is more straightforward, then that is used. Although, this is mostly a reason for keeping it, the that default is the thing least technically explained. But the main reason it is like this is due to this being the preference of my TTL at the time of writing this code).

[kbjarkefur]

Ok I am trying to test the stats() stuff but I can't find direct instructions. When I tried normd as in the help file I get:

The options nottest, normdiff, pttest, pftest and pboth have been deprecated as of version 7 of iebaltab. See if the options pairoutput or ftestoutput have the functionality you need. If you still need the old version of iebaltab, for example for reproduction of old code, see this guide for how to run old versions of any command in the ietoolkit package.

But there is no pairoutput since it is stats() now, right? The stats_string definitions don't seem to be in the help file yet so I can't test thoroughly, but I can back it out from the parser and use:

iebaltab trunk weight length , grpvar(rep78) savex("/users/bbdaniels/desktop/test.xlsx") covar(price) replace stats(pair(nrmd))

Then I get normalized differences for all comparisons. For these, we will replace by the formula above. I don't think these use significance stars in any case so those will drop as well -- the magnitude should be the only metric reported.

I think it is difficult enough to find time to work on the code, and while I will indeed update the helpfile before publishing, doing it for each version at this stage would make it even more difficult to find time to make progress with this command. This is why I have intentionally been slow with updating the helpfile. Feel free to ask on Slack for anything where you are so stuck you are not able to make progress.

Use my runfile to see how I intend to use the command: https://github.com/worldbank/ietoolkit/blob/iebaltab-rewrite-kb/run/iebaltab.do

[bbdaniels]

I was able to re-create the .v display error:

. iebaltab rep78 headroom trunk , grpv(rep78) control(1) savex("/users/bbdaniels/desktop/test.xlsx") stats(pair(nrmd)) replace

    Warning: All variance was explained by one variable in the regression for the pair-wise test of variable [rep78] for
        observations with values [2] and [1] in the groupvar [rep78].
.
    Warning: All variance was explained by one variable in the regression for the pair-wise test of variable [rep78] for
        observations with values [3] and [1] in the groupvar [rep78].
.
    Warning: All variance was explained by one variable in the regression for the pair-wise test of variable [rep78] for
        observations with values [4] and [1] in the groupvar [rep78].
.
    Warning: All variance was explained by one variable in the regression for the pair-wise test of variable [rep78] for
        observations with values [5] and [1] in the groupvar [rep78].

This should have some kind of better handling on output, unless it is supposed to be suppressed from exporting.

[bbdaniels]

Unless I'm mistaken, something has gone weird with the calculation or storage of standard errors. I didn't think I needed to do anything special to get basic stats. But I can't tell -- desperately need to write the help file!!

iebaltab length headroom trunk , grpv(rep78) savex("/users/bbdaniels/desktop/test.xlsx") replace

[kbjarkefur]

Unless I'm mistaken, something has gone weird with the calculation or storage of standard errors. I didn't think I needed to do anything special to get basic stats. But I can't tell -- desperately need to write the help file!!

iebaltab length headroom trunk , grpv(rep78) savex("/users/bbdaniels/desktop/test.xlsx") replace

You were not mistaken. There was a typo. Fixed in 7f8a363. Never has a bigger bug been solved with a smaller commit. 😄

(EDIT: It looked more fun in my client where only a single character was highlighted)

[kbjarkefur]

I was able to re-create the .v display error:

iebaltab rep78 headroom trunk , grpv(rep78) control(1) savex("/users/bbdaniels/desktop/test.xlsx") stats(pair(nrmd)) replace

I was able to reproduce this error. I have handled this error by testing that var in grpv() is not also in balance vars here: c558cdb

This fixes the error in your reproducible example. Did you get the .v display error in any other way than using a balance var as a group var?

[kbjarkefur]

@bbdaniels, in 2e1d71c I implemented what we decided with one difference.

It was too tricky to only add when it applied to the table note a reference about the missing values (.v, .o etc.) so I am always including it in the default table note. I think that is a fair compromise and will help this version get out the door without even further delays.