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

BUG/ENH: fix and improve constant detection #1797

Merged
merged 5 commits into from Jul 20, 2014

Conversation

Projects
None yet
2 participants
@josef-pkt
Copy link
Member

commented Jun 28, 2014

see #1794 and #1792

constant detection has now several layers of checks

  • (1) hasconst
  • (2) find column with ptp=0
    • find column with mean = 1
    • find column with mean !=0
  • (3) find implicit constant

a column of zeros does not count as constant anymore, const in (2) needs ptp==0 and mean !=0

disable try ... except, and handle exog is None explicitly (that was the only case that went trough the exception part

if we don't detect a non-zeros constant, then we check for implicit in all cases

(There are lot's of cases because I want to minimize or short circuit the cost of const detection.)

still open:

  • hasconst assumes implicit constant or no constant, cannot be used to specify const_idx
  • column of zeros prevents implicit constant check DONE
  • make it into standalone function and return (k_constant, const_idx) instead of attaching it directly.
  • incomplete cleanup, only cleanup left is to remove special code in RegressionModel
@coveralls

This comment has been minimized.

Copy link

commented Jun 28, 2014

Coverage Status

Coverage increased (+0.02%) when pulling 9a4e437 on josef-pkt:ref_hasconstant into 7df5291 on statsmodels:master.

@josef-pkt

This comment has been minimized.

Copy link
Member Author

commented Jun 29, 2014

@jseabold The last commit 38987ca removes the try except.

As far as I can see it was only covering the case for exog is None. For all other cases an exception was an error and should be fixed.

the previous diff view josef-pkt/statsmodels@statsmodels:master...38987ca
is slightly better, but mainly shows that instead of a few lines there are a lot more now.

any comments?

@coveralls

This comment has been minimized.

Copy link

commented Jun 29, 2014

Coverage Status

Coverage increased (+0.02%) when pulling 38987ca on josef-pkt:ref_hasconstant into 7df5291 on statsmodels:master.

@coveralls

This comment has been minimized.

Copy link

commented Jun 29, 2014

Coverage Status

Coverage increased (+0.03%) when pulling 38987ca on josef-pkt:ref_hasconstant into 7df5291 on statsmodels:master.

value = self.exog[:, idx].mean()
if value == 1:
self.k_constant = 1
self.const_idx = const_idx[idx]

This comment has been minimized.

Copy link
@josef-pkt

josef-pkt Jul 18, 2014

Author Member

shouldn't this be self.const_idx = idx ?

This comment has been minimized.

Copy link
@josef-pkt

josef-pkt Jul 18, 2014

Author Member

fixed in c5f2a04

@josef-pkt

This comment has been minimized.

Copy link
Member Author

commented Jul 18, 2014

should be ready to merge when TravisCI is green.

@coveralls

This comment has been minimized.

Copy link

commented Jul 18, 2014

Coverage Status

Coverage increased (+0.81%) when pulling 9a4366b on josef-pkt:ref_hasconstant into 7df5291 on statsmodels:master.

@josef-pkt

This comment has been minimized.

Copy link
Member Author

commented Jul 20, 2014

rebased and force pushed

@coveralls

This comment has been minimized.

Copy link

commented Jul 20, 2014

Coverage Status

Coverage increased (+0.03%) when pulling d888f7c on josef-pkt:ref_hasconstant into 88a4c51 on statsmodels:master.

josef-pkt added a commit that referenced this pull request Jul 20, 2014

Merge pull request #1797 from josef-pkt/ref_hasconstant
BUG/ENH: fix and improve constant detection

@josef-pkt josef-pkt merged commit 40bd7e0 into statsmodels:master Jul 20, 2014

2 checks passed

continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci The Travis CI build passed
Details

PierreBdR pushed a commit to PierreBdR/statsmodels that referenced this pull request Sep 2, 2014

Merge pull request statsmodels#1797 from josef-pkt/ref_hasconstant
BUG/ENH: fix and improve constant detection

@josef-pkt josef-pkt deleted the josef-pkt:ref_hasconstant branch Apr 10, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.