UX: add progress indicators for translation tasks#1257
UX: add progress indicators for translation tasks#1257jmartin-tech merged 3 commits intoNVIDIA:mainfrom
Conversation
Signed-off-by: Jeffrey Martin <jemartin@nvidia.com>
leondz
left a comment
There was a problem hiding this comment.
pretty elegant
one refactoring comment
garak/probes/continuation.py
Outdated
| preparation_bar = tqdm.tqdm( | ||
| total=len(self.triggers), | ||
| leave=False, | ||
| colour=f"#{garak.resources.theme.PROBE_RGB}", |
There was a problem hiding this comment.
considering different colour
| self.langprovider = self._get_langprovider() | ||
| if self.langprovider is not None and hasattr(self, "triggers"): | ||
| # check for triggers that are not type str|list or just call translate_triggers | ||
| preparation_bar = tqdm.tqdm( |
There was a problem hiding this comment.
for ease of maintenance i'd like a central location for these bars, maybe within langproviders or langservice, so we can update them all in one go. should probably be invoked from the current calling locations. one change i'd like is the name of the langprovider being used, to be in the description, for consistency - this seems easier to do w/ a centralised def. a tqdm object might do the trick.
There was a problem hiding this comment.
These are tqdm objects already.
I disagree with having the langprovider's name such as langprovider.remote.RivaTranslator in the description here, how does that improve the user's context on progress of the run? I could see some value in inserting the probe name since that is not displayed yet when this progress bar starts.
These should eventually be managed via a display or console presentation layer, I suggest we defer consolidation until we can build out a more consistent pattern.
The callback pattern used distinctly decouples the action taken from the notification that the service is triggering to make this easier to refactor later.
There was a problem hiding this comment.
IIRC, correct me if I'm wrong, progress bars in garak are typically labelled with a description of the class that's running and maybe some salient config too.
If we put in the probe name, do we give the experience of seeing a bar with a probe name complete while language provisioned, and then another bar with similar description start from zero again while prompts are sent to the generator?
suggest we defer consolidation until we can build out a more consistent pattern.
The pattern in this PR is pretty consistent, but repeated - can you say more about the expected level of consistency?
There was a problem hiding this comment.
can you say more about the expected level of consistency?
Consolidation of all the scattered tqdm usage into a display managing layer. The repeated pattern here is just two method calls, one to instantiate the bar and one to close it.
Existing progress bars often refer to the plugin that creates them or caused them to be created, in this case I can see mimic of how a tqdm for a buff that is mutating prompts is labeled:
desc=f"📥 Buffing probe: {probename}/{self.fullname}"
Translation is a very similar action so I could see expanding the specificity of this tqdm to be:
desc=f"Preparing triggers: {self.probename.replace("garak.", "")}"
There was a problem hiding this comment.
There are seven very similar tqdm.tqdm calls in this PR all covering language provision. My question is, ist this the time to establish a pattern for encapsulating these, given that style & descriptions are likely to be very similar also? Doesn't need to block, current code just looks a bit far from DRY
There was a problem hiding this comment.
These are all one method call that would need similar params passed to a new method, in my opinion this does to not make sense to refactor for this PR.
There was a problem hiding this comment.
Aight. It does feel marginal - almost all the params are things that one would specify to an encapsulation anyway. Will send descr & RGB const commits
Signed-off-by: Jeffrey Martin <jemartin@nvidia.com>
When a
target_langis specified some preparation prompts and triggers may batch work without providing any indicator to the console. To better communicate to the user the status of activity by the process provide a progress bar when processing these items in batch.Verification
List the steps needed to make sure this thing works
RIVA_API_KEY=fake python -m garak --config hf_RigoChat_gpu_riva.yml -m huggingface -n IIC/RigoChat-7b-v2 -g 1