-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Conversation
|
||
_counter = 0 | ||
|
||
def _global_mutable_counting(): |
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.
this counting makes it hard to do key matching of two independent running. For example, one is training and the other is applying.
better to add docstring |
The code looks almost identical to that of PyTorch. Can we have some diffchecker that can diff tensorflow directory against pytorch directory? To me, another draft PR overwriting the pytorch folder would be great. |
def undedup_mutables(self): | ||
return self._structured_mutables.traverse(deduplicate=False) | ||
|
||
def forward(self, *inputs): |
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.
I think forward is not needed for tensorflow.
self.reduction = reduction | ||
self.return_mask = return_mask | ||
|
||
def forward(self, *inputs): |
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.
call
?
self.reduction = reduction | ||
self.return_mask = return_mask | ||
|
||
def forward(self, optional_inputs): |
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.
same here
test_arc_per_epoch = 1 | ||
|
||
|
||
class EnasTrainer: |
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.
Is there an example ready for testing this trainer from end to end?
If no further improvement is planned for this PR, I'm strongly against merging it into master. Merging into a dev branch might be more acceptable. The main reason is, code currently presented in this PR seems to be never tested from end to end, which makes me doubt whether current design can actually work on tensorflow. |
* nas ui docs * add link in overview * update * Update Visualization.md
Merge v1.5 branch back to master
v1.5 release note
* update * update * update * update * update log message in bug-report template Co-authored-by: Lijiao <15910218274@163.com>
I will re-generate the commit history to make this patch clear. Expected commit order is shown as follow.
The items in itatic font are not yet ready.
Some features may be moved to future PRs.