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

Weighting Trajectories for Evaluation #84

Closed
1 task done
EthanJamesLew opened this issue Mar 8, 2024 · 15 comments
Closed
1 task done

Weighting Trajectories for Evaluation #84

EthanJamesLew opened this issue Mar 8, 2024 · 15 comments
Assignees

Comments

@EthanJamesLew
Copy link
Owner

EthanJamesLew commented Mar 8, 2024

Summary

  • add weights for each trajectory for evaluation

CC @Abdu-Hekal

@EthanJamesLew EthanJamesLew self-assigned this Mar 8, 2024
@Abdu-Hekal
Copy link
Collaborator

Abdu-Hekal commented Mar 9, 2024

Ideally for every N*M trajectory with N state variables and M time-steps, we could pass a N*M matrix of weights. So for a set of trajectories we have a set of these matrices. This would give use control over weights for each trajectory, state variable and time point

@EthanJamesLew
Copy link
Owner Author

@Abdu-Hekal does this code work for you?

@EthanJamesLew
Copy link
Owner Author

@Abdu-Hekal ping

@Abdu-Hekal
Copy link
Collaborator

@EthanJamesLew code runs well, Thank you!

@Abdu-Hekal
Copy link
Collaborator

@EthanJamesLew Is it possible to have the weights defined for each state variable? so that different state variables can have different weights?

@EthanJamesLew
Copy link
Owner Author

EthanJamesLew commented Mar 12, 2024

@Abdu-Hekal I believe that is doable as is (pass in a T x N x M array instead of T x M array), but lemme check

@Abdu-Hekal
Copy link
Collaborator

@EthanJamesLew I am getting the following error when I try it:
Error: operands could not be broadcast together with shapes (101,2) (101,)
I assume it multiplies each row of the variables difference by the full array of weights and not the corresponding row

@EthanJamesLew
Copy link
Owner Author

@Abdu-Hekal I see the issue--try now?

@Abdu-Hekal
Copy link
Collaborator

@EthanJamesLew Still getting the same error

@EthanJamesLew
Copy link
Owner Author

EthanJamesLew commented Mar 12, 2024

@Abdu-Hekal interesting. The GitHub CI/CD seemed to be okay with it: https://github.com/EthanJamesLew/AutoKoopman/actions/runs/8251423166/job/22568416799 (it ran the jupyter notebook with the updated example)

@Abdu-Hekal
Copy link
Collaborator

@EthanJamesLew Interesting, when I run the notebook I first get this error:
AttributeError: 'UniformTimeTrajectory' object has no attribute 'abs'.
Then I replace w = traj.abs().states with just w=abs(traj.states), and I run into the same error as before:
Error: operands could not be broadcast together with shapes (101,2) (101,)

@EthanJamesLew
Copy link
Owner Author

@Abdu-Hekal I did just add the abs method to trajectories objects, so maybe the autokoopman version being imported is not at HEAD. In any case, I know GH comments is not the best troubleshooting experience--maybe we could setup a brief pair programming session this week (I can do 8am PT Thu--the time we normally meet on Friday)?

@Abdu-Hekal
Copy link
Collaborator

@EthanJamesLew Sounds good! In the meantime I will try to fix the issue on my end.

@Abdu-Hekal
Copy link
Collaborator

@EthanJamesLew All good, Notebook Kernel just needed restarting :)
I am sometimes getting this warning though but does not seem detrimental
RuntimeWarning: overflow encountered in matmul
return np.real(self._A @ obs.T).flatten()[: len(x)]

@EthanJamesLew
Copy link
Owner Author

Great! Closing for now

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

No branches or pull requests

2 participants