-
Notifications
You must be signed in to change notification settings - Fork 0
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
Noise Benchmarking, LSTM sb3_zoo hyperpar, Issue #1 #9
Conversation
…nor changes to envs
…py now accepts new yaml syntax & old (TBD: re-standardize)
…sheries into benchmarking
Converted to draft to solve Issue #1 |
Updated based on our meeting! Also, side note: now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@felimomo fantastic work here! A few comments in line. Basically I think let's just add the harvest_vul in now as well, since it looks odd to have something explicitly called survey_vul
being used inside the harvest function, and then merge this.
I think we're due for a refactor of the Asm code, so that it's easier to read and easier to customize. (most importantly, asm_2o should re-use all the code except for an override observation method, and similarly for escapement with an override step method I think). but let's tackle that in a separate PR.
@@ -174,7 +173,7 @@ def initialize_population(self): | |||
|
|||
# leading array calculations to get vul-at-age, wt-at-age, etc. | |||
for a in range(0, p["n_age"], 1): | |||
vul[a] = 1 / (1 + np.exp(-p["asl"] * (p["ages"][a] - p["ahv"]))) | |||
survey_vul[a] = 1 / (1 + np.exp(-p["asl"] * (p["ages"][a] - p["ahv"]))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
once we have a harvest_vul
, it's going to need to be initialized here too. Presumably it's identical but has different values for p["asl"]
and p["ahv"]
. (We'll need @CarlJwalters or @ChrisFishCahill to figure out some reasonable choices there)
src/rl4fisheries/envs/asm.py
Outdated
self.abar = sum(p["vul"] * np.array(p["ages"]) * n) / sum(n) | ||
self.wbar = sum(p["vul"] * n * p["wt"]) / sum(n * p["wt"]) | ||
self.abar = sum(p["survey_vul"] * np.array(p["ages"]) * n) / sum(n) | ||
self.wbar = sum(p["survey_vul"] * n * p["wt"]) / sum(n * p["wt"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above, eventually this should be harvest_vul
In this branch:
Asm().sigma
) with a script input, added a bash script to train agents for several values of noise simultaneously.Next steps: sb3_train_v2 is more general than sb3_train, however its syntax is a bit more involved and might need refactoring in order to reach sb3_train's level of clarity. I'll leave this for the next pull request -- the code is fully functional right now, although in a 'quick-n-dirty' state in some parts.