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

forests get Type, so Predict can be generic. #25

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

forests get Type, so Predict can be generic. #25

wants to merge 8 commits into from

Conversation

glycerine
Copy link

Early draft, but almost done:

  • draft PR posted for feedback (wait to merge).
  • still todo: read back in the ForestType from the serialized text file.
  • test that Predict() computes the same answers as applyforrest

@glycerine
Copy link
Author

Serialization part is done.

Needs review and in particular the decoding of options into the ballot box type needs comparison with the applyforest.go logic.

@glycerine
Copy link
Author

I made a stand alone command,cfpred, that exercises the Predict() functionality.

Initial comparison to applyforest on the iris and foresfires data set, under classifier and -gbt regression showed identical predictions.

@glycerine
Copy link
Author

Okay. This is ready to review, and if acceptable, merge.

  • Test of Predict() in place in pred_test.go.
  • To reduce risk, I've kept the newlogic in cfpred and left applyforest the same. A bit conservative, but I don't want to mess up current users/uses of applyforest. cfpred should be the way forward for new development, and should eventually replace applyforest once it matures.

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