Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

Bug fix for Cream NAS #3498

Merged
merged 1 commit into from
Apr 6, 2021
Merged

Conversation

alibaba-yiwuyao
Copy link
Contributor

Two bugs may be fixed for the NAS (Cream of the Crop):

  1. in "nni/nni/algorithms/nas/pytorch/cream/trainer.py", line 168:
    I think the prioritized board should be updated with the newest student network, not the teacher. Otherwise, the prioritized board will be filled with the same teacher?

  2. in "nni/examples/nas/cream/lib/models/structures/supernet.py", line 126:
    In the class method rand_parameters(self, ...), when yielding parameters of the block in mutable, the passed architecture is the dictionary with the type Dict[str, np.array], of which the key is mutable.key, and the value is a one-hot vector. In the original function, the traversed blocks is already a block in the mutable, which can not be indexed by the arch. Therefore, the class method is modified by using (block, arch) to find the sampled block in the mutable.

@ghost
Copy link

ghost commented Mar 30, 2021

CLA assistant check
All CLA requirements met.

@alibaba-yiwuyao
Copy link
Contributor Author

@penghouwen @Z7zuqer, Hi, can you help to review this pull request? thanks:)

@QuanluZhang QuanluZhang requested a review from penghouwen March 31, 2021 01:27
@alibaba-yiwuyao
Copy link
Contributor Author

Hi, @penghouwen @Z7zuqer ~~
The cream NAS is an excellent work by using inter-candidate distillation during one-shot search. And we have tried to apply this technique to our mobile solution for facial landmark, needing extremely fast running speed on Qualcomm 625 chip.
However, we find some possible issues listed in this Pull Request. We need your help to check this PR, which is helpful for our ongoing work, thx:)

@Z7zuqer
Copy link
Contributor

Z7zuqer commented Apr 2, 2021

Hi, @penghouwen @Z7zuqer ~~
The cream NAS is an excellent work by using inter-candidate distillation during one-shot search. And we have tried to apply this technique to our mobile solution for facial landmark, needing extremely fast running speed on Qualcomm 625 chip.
However, we find some possible issues listed in this Pull Request. We need your help to check this PR, which is helpful for our ongoing work, thx:)

Hi,

Thanks for your interest in our project!

Two bugs you mention in this PR are indeed in our current codebase, I think it's ok to merge this PR.

Best,
Hao.

@alibaba-yiwuyao
Copy link
Contributor Author

Hi, @QuanluZhang ~
Can you help to add Z7zuqer as the reviewer? Thanks:)

@QuanluZhang
Copy link
Contributor

QuanluZhang commented Apr 2, 2021

Hi, @QuanluZhang ~
Can you help to add Z7zuqer as the reviewer? Thanks:)

sorry, only the account that joined microsoft org can be assigned as reviewer i guess. I think @Z7zuqer will help review :)

@QuanluZhang
Copy link
Contributor

@Z7zuqer if you think this pr is ok, please approve it

@Z7zuqer
Copy link
Contributor

Z7zuqer commented Apr 2, 2021

@Z7zuqer if you think this pr is ok, please approve it

Hi,

I think it's ok to merge this PR, but I am not the reviewer, could I ask @penghouwen to help me approve this?

Best,
Hao.

@QuanluZhang
Copy link
Contributor

@Z7zuqer if you think this pr is ok, please approve it

Hi,

I think it's ok to merge this PR, but I am not the reviewer, could I ask @penghouwen to help me approve this?

Best,
Hao.

you can approve without being assigned as an reviewer :)

@QuanluZhang QuanluZhang requested a review from ultmaster April 2, 2021 07:22
@penghouwen
Copy link
Contributor

@Z7zuqer if you think this pr is ok, please approve it

Hi,
I think it's ok to merge this PR, but I am not the reviewer, could I ask @penghouwen to help me approve this?
Best,
Hao.

you can approve without being assigned as an reviewer :)

Approved. @AliCloud-PAI Thanks for your interest and kind helps on Cream!

@alibaba-yiwuyao
Copy link
Contributor Author

Hi, @QuanluZhang ~~
Two reviews are approved, but the merging is still blocked, can you help to check?
Thanks a lot:)

@QuanluZhang QuanluZhang merged commit d6b5c6c into microsoft:master Apr 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants