Skip to content
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

add initial pass at arg_constraints and arg validation. #123

Merged
merged 1 commit into from
Jan 30, 2022

Conversation

erip
Copy link
Contributor

@erip erip commented Dec 28, 2021

Fixes #120.

@erip
Copy link
Contributor Author

erip commented Dec 28, 2021

I'm a bit sketchy on constraints for some of the parameters which get overloaded as tuples (i.e., log_potentials in SentCFG), so review on this seems particularly relevant. Everything else is pretty vanilla.

@srush
Copy link
Collaborator

srush commented Dec 28, 2021

SentCFG is a tuple of reals (triple). Does that work with constraints?

@erip
Copy link
Contributor Author

erip commented Dec 28, 2021

I guess that's the question. It seems like typically the expectation is parameters are not composite so you can have a clean mapping of parameter -> constraint. Since log_potentials is still technically "a real" (just in higher dim space) it should be ok... I can look into adding a test to see if we can validate.

@erip
Copy link
Contributor Author

erip commented Dec 28, 2021

As an aside, is there a reason CI didn't run on this PR? The GH Actions config looks OK so I'm not sure why I can't see the results of the tests.

@srush
Copy link
Collaborator

srush commented Dec 29, 2021

Kicked off the CI.

@srush srush self-requested a review January 30, 2022 19:04
@srush srush closed this Jan 30, 2022
@srush srush reopened this Jan 30, 2022
@srush srush merged commit 7146de5 into harvardnlp:master Jan 30, 2022
@erip erip deleted the fix/add_constraints branch January 30, 2022 19:56
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.

pytorch-struct shows a warning for torch versions >= 1.8.0
2 participants