-
Notifications
You must be signed in to change notification settings - Fork 104
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
[Add Randomized SVD in PCA] #298
Conversation
sml/pca/simple_pca_emul.py
Outdated
@@ -59,17 +61,17 @@ def proc_reconstruct(X): | |||
) | |||
emulator.up() | |||
# Create a simple dataset | |||
X = random.normal(random.PRNGKey(0), (15, 100)) | |||
X = random.normal(random.PRNGKey(0), (1000, 10)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
请 emulator run 之前请先 seal 要处理的数据
sml/pca/simple_pca.py
Outdated
|
||
class SimplePCA: | ||
def __init__( | ||
self, | ||
method: str, | ||
n_components: int, | ||
max_iter: int = 100, | ||
n_iter: int = 4, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里有两个iter数的超参,感觉需要从语意上明确一下,如:
max_iter -> power_iter
n_iter -> projection_iter
sml/pca/simple_pca.py
Outdated
|
||
class SimplePCA: | ||
def __init__( | ||
self, | ||
method: str, | ||
n_components: int, | ||
max_iter: int = 100, | ||
n_iter: int = 4, | ||
random_state: int = 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
random_state不需要作为参数
sml/pca/simple_pca.py
Outdated
""" | ||
assert len(X.shape) == 2, f"Expected X to be 2 dimensional array, got {X.shape}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个shape check应该是都需要的吧,没必要移到power method里面
sml/pca/simple_pca.py
Outdated
# Remove the component from the covariance matrix | ||
cov_matrix -= eigval * jnp.outer(vec, vec) | ||
elif self._method == Method.PCA_rsvd: | ||
random_state = np.random.RandomState(self._random_state) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
随机矩阵不能在函数里生成,可以考虑在fit里增加一个入参
sml/pca/simple_pca.py
Outdated
@@ -56,71 +72,105 @@ def __init__( | |||
self._mean = None | |||
self._components = None | |||
self._variances = None | |||
self._n_iter = n_iter # used in rsvd | |||
self._random_state = random_state # used in rsvd | |||
self._scale = scale # used in rsvd | |||
self._method = Method(method) | |||
|
|||
def fit(self, X): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
随机矩阵考虑增加到入参里
另外,有几个点麻烦您注意一下哈:
感谢~ |
What problem does this PR solve?
使用 SPU 优化 PCA 算法
Issue Number: Fixed #259
Possible side effects?