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

Question type classification #451

Merged
merged 10 commits into from
Dec 25, 2016
Merged

Question type classification #451

merged 10 commits into from
Dec 25, 2016

Conversation

danyaljj
Copy link
Member

@danyaljj danyaljj commented Dec 6, 2016

Adding application: Question Type Classification. Details in the readme.

@danyaljj danyaljj requested a review from bhargav December 8, 2016 21:53
Copy link
Contributor

@bhargav bhargav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I'll merge tomorrow.

FYI @kordjamshidi

@kordjamshidi
Copy link
Member

It seems I do not have write access and permission to merge? @bhargav

@bhargav
Copy link
Contributor

bhargav commented Dec 24, 2016

@kordjamshidi Strange. You had Admin permissions already. Anyways I also made the Saul team to have Admin privileges to the repository. Do you have write access/permissions now.

@kordjamshidi
Copy link
Member

I see that I have the permissions now, thanks. This was not the case the other day.

@bhargav bhargav merged commit 55a9735 into CogComp:master Dec 25, 2016
@@ -245,7 +245,7 @@ trait DataModel extends Logging {
def apply(f: T => List[String])(implicit tag: ClassTag[T], d1: DummyImplicit, d2: DummyImplicit, d3: DummyImplicit,
d4: DummyImplicit, d5: DummyImplicit, d6: DummyImplicit): DiscreteCollectionProperty[T] = {
def cachedF = if (cache) { x: T => getOrUpdate(x, f).asInstanceOf[List[String]] } else f
val a = new DiscreteCollectionProperty[T](name, cachedF, ordered) with NodeProperty[T] {
val a = new DiscreteCollectionProperty[T](name, cachedF, !ordered) with NodeProperty[T] {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danyaljj why the order negated here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bug that existed in the system from a long time ago and I noticed it when I was making these changes (also mention in #454).
But fixing it required retraining systems, which at the time I didn't have time to do it. So I left it as an issue.

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.

4 participants