Using Pydelphin for Test by Generation

delph-in / matrix

The Grammar Matrix

https://matrix.ling.washington.edu/index.html

Other

11 stars 6 forks source link

Using Pydelphin for Test by Generation #663

Closed rosypen closed 1 year ago

rosypen commented 1 year ago

These changes reduce the code complexity considerably thanks to @goodmami for the pointers on how to use Pydelphin to call ACE for generation.

We now have parse trees and well formatted MRS on display!

Up Next:

Parsing the Test Sentences in the choices file and generating from those MRS
Specifying max number of sentences to generate

The above two tasks will involve changing the Test by Generation Options page.

rosypen commented 1 year ago

Oh I made another commit which I didn't realize would be automatically added to this pull request.

The new commit enables the feature to generate sentences using the test sentences in the choices file

Enable Generate from Test Sentences

Please click on the link above to see the full workflow of how that works. But In summary, we have this now:

emilymbender commented 1 year ago

I'm happy to merge these, but I'd love some advice from those more familiar with github ... if I just click "merge pull request" on the latest, does it all get merged? Is there anything I need to be careful about?

arademaker commented 1 year ago

I would suggest the Squash and Merge option so all intermediary commits in the source branch will be collapsed into a single commit in the trunk branch.

arademaker commented 1 year ago

Sorry, ignore the link I added in the last message; I just edited the message to fix it; I was trying to copy a screenshot.

arademaker commented 1 year ago

It is an excellent practice always to have an issue associated with a PR, so the issue document the discussion that eventually was solved with a PR.

goodmami commented 1 year ago

@emilymbender quick answer: yes, if you merge it will merge all the commits in this PR to the target branch (which is trunk here). If the target branch has changed since the PR was created and has introduced conflicts, GitHub would prevent you from merging until the conflicts were addressed.

I agree with @arademaker's suggestion to always have an issue for a PR. The issue describes the problem, and the PR describes the solution. When you close the issue via the PR, they get linked, and they both become a historical record of the development.

I don't recommend "Squash and merge" until you feel comfortable with Git and maybe not even then. It's a little more risky because it changes the history (local to the PR) which can introduce hard-to-fix conflicts. The following article lays out some pros/cons of the merge options: https://www.lloydatkinson.net/posts/2022/should-you-squash-merge-or-merge-commit/

emilymbender commented 1 year ago

Thank you all --- I have merged this!