Skip to content

Conversation

@JohnConnor123
Copy link

@JohnConnor123 JohnConnor123 commented Feb 15, 2025

Pull Request Checklist

Reference Issue

Please provide the reference to issue this PR is addressing (# followed by the issue number). If there is no associated issue, write "N/A".

ref: #182

Checklist Items

Before submitting your pull request, please review these items:

  • Have you followed the contributing guidelines?
  • Have you verified that there are no existing Pull Requests for the same update/change?
  • Have you updated any relevant documentation or added new tests where needed?

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Documentation content changes
  • Other...
    • Description:

window_size: Optional[int] = kwargs.get("window_size", None)

name = self._model_coordinator.get_output_filename(
top_k_candidates, dataset_name, shuffle_candidates, **kwargs
Copy link
Member

Choose a reason for hiding this comment

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

Please change it within get_output_filename functions in all the rerankers in case they are/will be called from other places.
If you define a helper function (get_formatted_datetime or something similar) in the root RankLLM class, then all the get_output_filename implementations can call that.

Please also do a git grep to find datetime to see if we have this problem anywhere else, since ":" in the name comes from the datetime.isoformat that we append to the output files.

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