Skip to content

Conversation

@anvo2
Copy link
Collaborator

@anvo2 anvo2 commented Dec 11, 2024

Might recode the particles into a separate dim if necessary

Copy link
Member

@AlexanderFengler AlexanderFengler left a comment

Choose a reason for hiding this comment

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

left some comments that need to be addressed

Copy link
Member

Choose a reason for hiding this comment

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

this file shouldn't sit in this particular folder

Copy link
Member

Choose a reason for hiding this comment

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

(not sure if it should exist at all)

},
"ddm_seq2_no_bias_race2": {
"name": "ddm_seq2_no_bias_race2",
"params": ["vha", "vhb", "vl1a", "vl1b", "vl2a", "vl2b" , "a", "t"],
Copy link
Member

Choose a reason for hiding this comment

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

the v settings are a bit aggressive.
maybe -3.0, 3.0?

Was there a reason to set them to -4, 4?

Copy link
Member

Choose a reason for hiding this comment

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

Looking over the simulator, moreover in this context you want the bounds on v parameters generally to have a lower bound of 0.

In the race model version, negative drifts don't really make sense (not in our context at least).

float max_t = 20,
int n_samples = 20000,
int n_trials = 1,
print_info = True,
Copy link
Member

Choose a reason for hiding this comment

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

print_info is an option only for this simulator right?
In which case I would skip it for consistency.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

print_info = True is also there on the other 4-choice simulators

@AlexanderFengler
Copy link
Member

Tests also failing, needs investigation.

@AlexanderFengler
Copy link
Member

@anvo2 ping on this. Comments being addressed?

@AlexanderFengler
Copy link
Member

@anvo2 the tests are failing still.
Is this WIP or supposed to be ready for review?

Copy link
Member

@AlexanderFengler AlexanderFengler left a comment

Choose a reason for hiding this comment

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

@anvo2 just address the remaining issues then we can merge this.

Copy link
Collaborator Author

@anvo2 anvo2 left a comment

Choose a reason for hiding this comment

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

I believe everything is all set?

an_vo@brown.edu and others added 2 commits April 19, 2025 16:09
@anvo2
Copy link
Collaborator Author

anvo2 commented Jun 11, 2025

please ignore the entire block of seq2_short. That will get deleted out.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

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