-
Notifications
You must be signed in to change notification settings - Fork 4
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
nettack #17
nettack #17
Conversation
requirements3.txt
Outdated
# google--async-resumable-media | ||
# google--upb | ||
google-ai |
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.
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.
Не вижу каких-то ошибок, по большей части прокомментил на уровне косметических моментов. Всё ок
|
||
|
||
def largest_connected_components(adj, n_components=1): | ||
"""Select the largest connected components in the graph. |
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.
Кажется, надо подфиксить документацию
- Что такое gust? У нас такой либы нет, вроде -> не очень понятно, что всё же на вход ожидается в плане формата
- adj -> sparse_graph просто не поменял название
|
||
|
||
def train_w1_w2(dataset, hidden): | ||
data = dataset.data |
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.
Вот тут dataset - это же gen_dataset, иными словами - наша сущность вида датасет менеджер? С Кириллом обсуждали, что наиболее безопасное прокидывание проперти через dataset.dataset.data в таком случае (ну и далее по тексту, если будут, такие же обращения). Оно может и так прокинуться, но может и не прокинуться, поэтому, если я не перепутал ничего и dataset.dataset.data работает - лучше так.
P.S. хотя, конечно, ещё лучше, если в будущем у нас будет удобный геттер...
|
||
return self.X_obs.dot(self.W) | ||
|
||
def get_attacker_nodes(self, n=5, add_additional_nodes = False): |
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.
если будет не влом и будешь какие-то изменения по итогу вносить, тут пробелы по PEP лишние, глаза режет)
Мелочь, но если вдруг, почему бы заодно не пофиксить
def attack_diff(self): | ||
return self.attack_diff | ||
|
||
@staticmethod |
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.
Тут точно нужен статик? Подразумевается, что возможно применение метода вне NetEvasionAttacker?
Add nettack attack implementation. Code taken from author danielzuegner/nettack and adapted to our framework