8000 Update `sem_map` by vincentzed · Pull Request #173 · lotus-data/lotus · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Update sem_map #173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Update sem_map #173

wants to merge 9 commits into from

Conversation

vincentzed
Copy link

Testing strategy:
pytest .github/tests/lm_tests.py::test_sem_map_nsample .github/tests/lm_tests.py::test_sem_map_nsample_with_returns .github/tests/lm_tests.py::test_sem_map_temp -v

Another idea I've been meaning to get to is adding a sample parameter, that let's you generate multiple columns from the same langex. a good starting point could be with map eg:
df.sem_map("what are the main ideas of {article}", nsample=10, temp=1.0)

model_kwargs = {"progress_bar_desc": progress_bar_desc}

# Add sampling parameters
model_kwargs["n"] = nsample
Copy link
Author
@vincentzed vincentzed Apr 23, 2025

Choose a reason for hiding this comment

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

Pyright checks:

/Users/mrnoodles/Github/Open Source/lotus/lotus/sem_ops/sem_map.py:69:5 - error: Argument of type "int" cannot be assigned to parameter "value" of type "str" in function "__setitem__"
    "int" is not assignable to "str" (reportArgumentType)
  /Users/mrnoodles/Github/Open Source/lotus/lotus/sem_ops/sem_map.py:72:9 - error: Argument of type "float" cannot be assigned to parameter "value" of type "str" in function "__setitem__"
"float" is not assignable to "str" (reportArgumentType)"

I think this is ok since it's model kwargs, but I can do some digging to fix

@liana313 liana313 requested a review from harshitgupta412 May 5, 2025 18:47
@liana313
Copy link
Collaborator
liana313 commented May 5, 2025

Nice work on this! It would make sense to take an optional temperature argument here so that the user doesn't have to reset it manually by reconfiguring the LM

@vincentzed vincentzed marked this pull request as ready for review May 6, 2025 11:59
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
0