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

gegen group misc issues #4

Closed
sergiocorreia opened this issue Jul 25, 2017 · 4 comments
Closed

gegen group misc issues #4

sergiocorreia opened this issue Jul 25, 2017 · 4 comments
Assignees

Comments

@sergiocorreia
Copy link

A few issues with gegen:

  • It doesn't seem to respect the order of the data. EG:
sysuse auto, clear
egen id1 = group(turn trunk)
fegen id2 = group(turn trunk)
gegen id3 = group(turn trunk)

assert id1==id2
assert id1==id3

The last command fails as the IDs are not the same

  • It doesn't warn for incorrect options:
gegen id = group(turn) , wrongoption

Should give a warning that an incorrect option was used.

@mcaceresb
Copy link
Owner

I note in the README that gegen does not respect group order; perhaps I should change the function name since you find it misleading? (I'll fix the wrong option bug)

@sergiocorreia
Copy link
Author

Another problem is that the new IDs seem to have holes:

sysuse auto, clear
gegen id = group(turn) in 20/30
tab id

(groups 1-4, 6, 8, etc. are missing)

@mcaceresb
Copy link
Owner

I see. This is not a but per se, though, since all the groups are there and the ID is unique, but I agree that coupled with the order issue it makes the ID rather confusing.

@mcaceresb mcaceresb self-assigned this Jul 26, 2017
mcaceresb added a commit that referenced this issue Jul 27, 2017
Enhancements

* `gegen varname = group(varlist)` no longer has holes, as noted in issue
  #4
* `gegen` and `gcollapse` fall back on `collapse` and `egen` in case there
  is a collision. Future releases will implement an internal way to resolve
  collisions. This is not a huge concern, as SpookyHash has no known
  vulnerabilities (I believe the concern raied in issue #2
  was base on a typo; see [here](rurban/smhasher#34))
  and the probability of a collision is very low.
* `gegen varname = group(varlist)` now has a consistency test (though
  the group IDs are not the same as `egen`'s, they should map to the `egen`
  group IDs 1 to 1, which is what the tests now check for).

Bug fixes

* Additional fixes for issue #1
* Apparentlly the argument Stata passes to plugins have a maximum length. The
  code now makes sure chuncks are passed when the PATH length will exceed the
  maximum. The plugin later concatenates the chuncks to set the PATH correctly.
mcaceresb added a commit that referenced this issue Jul 27, 2017
Bug fixes

* `gegen` no longer ignores unavailable options, as noted in issue
  #4, and now it throws an error.
mcaceresb added a commit that referenced this issue Jul 27, 2017
gtools-0.6.5 through gtools-0.6.9

Enhancements

* Addressed the possible issue noted in issue
  #3 and the functions now
  use mata and extended macro functions as applicable.
* `gegen varname = group(varlist)` no longer has holes, as noted in issue
  #4
* `gegen` and `gcollapse` fall back on `collapse` and `egen` in case there
  is a collision. Future releases will implement an internal way to resolve
  collisions. This is not a huge concern, as SpookyHash has no known
  vulnerabilities (I believe the concern raied in issue #2
  was base on a typo; see [here](rurban/smhasher#34))
  and the probability of a collision is very low.
* `gegen varname = group(varlist)` now has a consistency test (though
  the group IDs are not the same as `egen`'s, they should map to the `egen`
  group IDs 1 to 1, which is what the tests now check for).
* The function now checks numerical variabes to see if they are integers.
  Working with integers is faster than hashing.
* The function is now smarter about generating targets. In prior versions,
  when the target statistic was a sum the function would force the target
  type to be `double`. Now if the source already exists and is a float, the
  function now checks if the resultimg sum would overflow. It will only
  recast the source as double for collapsing if the sum might overflow, that
  is, if `_N * min < -10^38` or `10^38 < _N * max` (note +/- 10^38 are the
  largest/smallest floats stata can represent; see `help data_types`).

Bug fixes

* `gegen` no longer ignores unavailable options, as noted in issue
  #4, and now it throws an error.
* `gegen varname = tag(varlist)` no longer tags missing values, as noted
  in issue #5
* Additional fixes for issue #1
* Apparentlly the argument Stata passes to plugins have a maximum length. The
  code now makes sure chuncks are passed when the PATH length will exceed the
  maximum. The plugin later concatenates the chuncks to set the PATH correctly.
* Fixed issue #1
* The problem was that the wrapper I wrote to print to the Stata
  console has a maximum buffer size; when it tries to print the
  new PATH it encounters an error when the string is longer than
  the allocated size. Since printing this is unnecessary and
  will only ever be used for debugging, I no longer print the PATH.
* Debugging issue #1
  on github (in particular, `env_set` on Windows).
* Removed old debugging code that had been left uncommented
* Improved out-of-memory message (now links to relevant help section).
@mcaceresb
Copy link
Owner

The IDs should no longer have have holes, and extraneous options now give an error. See 0.6.10 (master branch). However, the order of the IDs is pending a future release. I am opening a new issue for that. (See issue #6).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants