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 tests for reward function for one timesteps #17

Merged
merged 6 commits into from
Aug 28, 2024
Merged

Add tests for reward function for one timesteps #17

merged 6 commits into from
Aug 28, 2024

Conversation

jiangjingzhi2003
Copy link
Contributor

added test for reward function for one timesteps

  1. no trap laid when no crabs
  2. all trap laid when no crabs
  3. no trap laid when there is large amount of crabs
  4. all trap laid when there is large amount of crabs

@cboettig
Copy link
Member

Nice work @jiangjingzhi2003 ! @felimomo can you take a quick look over this when you have a chance?

@felimomo
Copy link
Collaborator

Awesome progress @jiangjingzhi2003 !! I added a few comments, let me know what you think. (Tomorrow I'll be working offline from the plane—I might not be super responsive, but I'll get back to you on Wednesday!)

@jiangjingzhi2003
Copy link
Contributor Author

Awesome progress @jiangjingzhi2003 !! I added a few comments, let me know what you think. (Tomorrow I'll be working offline from the plane—I might not be super responsive, but I'll get back to you on Wednesday!)

I don't think I can see your comments. Where did you put it?

@felimomo
Copy link
Collaborator

@jiangjingzhi2003 they should appear as threads here! #17 or in the files changed https://github.com/boettiger-lab/rl4greencrab/pull/17/files

@jiangjingzhi2003
Copy link
Contributor Author

jiangjingzhi2003 commented Aug 26, 2024

Screenshot 2024-08-26 at 16 03 42 @felimomo It is empty from my end.

@cboettig
Copy link
Member

@felimomo i think github waits until you hit 'finish review' to post in-line comments

tests/test_methods.py Outdated Show resolved Hide resolved
tests/test_methods.py Outdated Show resolved Hide resolved
tests/test_methods.py Show resolved Hide resolved
@felimomo
Copy link
Collaborator

Ah thanks Carl, I had completely blanked out on 'hitting send'—sorry about that Jim!

@felimomo
Copy link
Collaborator

Hey, was closing it intentional? I think it's not merged to main yet!

@jiangjingzhi2003
Copy link
Contributor Author

jiangjingzhi2003 commented Aug 28, 2024

Oh, my bad. My closed it by accident. I will reopen it.

@felimomo felimomo merged commit 8633ce5 into main Aug 28, 2024
6 checks passed
@cboettig
Copy link
Member

cboettig commented Aug 28, 2024

@felimomo I think GitHub should be giving you the option to "approve" the review after the issues you flagged are resolved. I think we've set it up so only one reviewer needs to review and approve and then you can merge.

Thanks both for your work on this!

I think I'll make students in ESPM-157 learn to review and approve PRs this year -- hope they don't throw me overboard! 🏴‍☠️

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.

3 participants