Skip to content

Conversation

@klchen0112
Copy link

PR Type and Checklist

What kind of change does this PR introduce?

When saving a large number of samples, saving each batch repeatedly will consume a lot of time.
I added an option to control whether generated samples are saved every epoch.

  • FIX BUG
    • Check CURRENT and PREVIOUS ISSUES..
    • Check WORFLOW result after PR.
  • ADD ATTACK
    • DEFAULT values of arguments should be given except model.
    • Classname should be given as CamelCase.
    • Python file name should be SMALL letters of the classname.
    • Add attack in init.py
    • Check supported_mode whether the attack supports targeted mode.
    • README.md should be updated.
    • Check WORFLOW result after PR.
  • Other... Please describe:

Copy link
Contributor

@rikonaka rikonaka left a comment

Choose a reason for hiding this comment

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

There seems to be a logical problem with some of the code ❤️

return inputs

@staticmethod
def _save_adv_examples(
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps adding a type constraint or default parameters value for each parameter here would help other maintainers have a clear definition of parameter types.

save_dict = {
"adv_inputs": adv_input_list_cat,
"labels": label_list_cat,
} # nopep8
Copy link
Contributor

Choose a reason for hiding this comment

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

Here # nopep8 can be removed.


save_dict["save_type"] = save_type
torch.save(save_dict, save_path)
if save_path is not None and save_every_iter:
Copy link
Contributor

Choose a reason for hiding this comment

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

The input parameters of these two self._save_adv_examples functions seem to be the same, so why are two placed here?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants