search

infer-actively / pymdp

A Python implementation of active inference for Markov Decision Processes
MIT License
420 stars 83 forks source link

Proposed fixes to control.py and utils.py #56

Open spetey opened 2 years ago

spetey commented 2 years ago

1) control.py's action_marginals should sum only over the first time step, 2) utils.py's sample(probabilities) throws an error with the argument "squeezed"

conorheins commented 2 years ago

thanks for this @spetey -- before I run the CI test-suite, can you please remove the extra files from your commit (those ones committed besides utils.py and control.py)? That way we don't clog up the repo with additional files

spetey commented 2 years ago

Ugh, I've tried to reset to HEAD~1 and ignore the "egg-info" directory that was auto-added somehow, but then there were conflicts because the upstream repo had changed, and then it stopped ignoring the "egg-info" ... I find git terribly confusing, I tried to fix it as best I could, and opened another pull request. Thanks again for your patience.

Steve

On Wed, Dec 15, 2021 at 5:43 AM conorheins @.***> wrote:

thanks for this @spetey https://github.com/spetey -- before I run the CI test-suite, can you please remove the extra files from your commit (those ones committed besides utils.py and control.py?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/infer-actively/pymdp/pull/56#issuecomment-994664478, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPMDVDQOFJVG4TKWR7KA4TURBWLDANCNFSM5KCQFIIA .

conorheins commented 2 years ago

Hey @spetey I've pushed another branch for the documentation that also included the fix to control.py already, so we don't need that commit anymore. So all that remains is to figure out the utils.sample() function, that you're proposing a change to. Let me know what the error is and we can figure out whether dropping that .squeeze() is the right way forward.

alec-tschantz commented 2 years ago

Ugh, I've tried to reset to HEAD~1 and ignore the "egg-info" directory that was auto-added somehow, but then there were conflicts because the upstream repo had changed, and then it stopped ignoring the "egg-info" ... I find git terribly confusing, I tried to fix it as best I could, and opened another pull request.

I've added a gitignore *.egg.info to a PR, so this won't be an issue going forward