Skip to content

Add Linear Regression to Aggregator #262

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

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

Conversation

giangnm58
Copy link

No description provided.

@hridesh
Copy link
Member

hridesh commented Apr 10, 2020

Do we have new test cases?

@hridesh hridesh changed the title add Linear Regression to Aggregator Add Linear Regression to Aggregator Apr 10, 2020
@hridesh hridesh requested a review from psybers April 10, 2020 00:55
// for ML
public static String HADOOP_SEQ_FILE_LOCATION = "";
public static String HADOOP_OUT_LOCATION = "";
//public static String HADOOP_OUT_LOCATION = "/home/tess/Desktop/datagen";
Copy link
Member

Choose a reason for hiding this comment

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

Are these changes necessary?

Copy link
Author

@giangnm58 giangnm58 Apr 10, 2020

Choose a reason for hiding this comment

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

Currently, it is used to store the trained model.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps the commented out code on line 80 can be removed.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I have removed this line.

@@ -0,0 +1,11 @@
package boa;
Copy link
Member

Choose a reason for hiding this comment

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

What is the relation between BoaTup and BoaTuple, a class that already exists?

Copy link
Author

Choose a reason for hiding this comment

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

BoaTup is used to read the training data.

Copy link
Member

Choose a reason for hiding this comment

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

Please see src/java/boa/types/BoaTuple.java. Can that existing type serve this purpose?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not even sure this is being used.

There are no classes inheriting from this interface. There are no instances of this type being created anywhere.

I think this file, and other code referencing it (like the functions that accept a BoaTup as argument), can probably be removed?

Copy link
Author

Choose a reason for hiding this comment

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

Currently, I am still using BoaTup for the training data. I have added the function which uses BoaTup. This function will be used to classify the data from trained model.

@giangnm58
Copy link
Author

Do we have new test cases?
No, I did not create test cases for linear regression aggregator.

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

Successfully merging this pull request may close these issues.

4 participants