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

UC Davis OptiGAN Bug Fix #574

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

guneet19
Copy link
Contributor

Changed OpitGAN code to reflect steps_to_store = "first".

steps_to_store = "entering" leads to incorrect output from OptiGAN. Even filtering the root output gives wrong results.

@nkrah Is it possible to default to steps_to_store = "first" when using OptiGAN?

@nkrah
Copy link
Collaborator

nkrah commented Nov 19, 2024

Good idea to add the possibility to override the default values coming from the base class. I'll implement that.

@guneet19
Copy link
Contributor Author

@nkrah If you haven't started working on this already. We have a member in our team who can start working on it. He is getting trained on Gate 10 and can be a good starting point for contribution. For now, I will add a doc stating to use steps_to_store = "first" when using OptiGAN. In that case, this code can be merged if there are no conflicts.

@nkrah
Copy link
Collaborator

nkrah commented Nov 21, 2024

@guneet19 : It's on my list, but I have not started yet. Pushing the user guide forward is the priority right now.

If you can add a line in the doc, as you suggest, that would be good.

Otherwise, thanks for the offer. Handling the user infos is actually quite deep down in the class factory mechanics used for GateObjects. I don't think that is a good starting point for somebody being trained on GATE 10.

nkrah added a commit that referenced this pull request Nov 29, 2024
@guneet19
Copy link
Contributor Author

We are working on updated version of optiGAN and found few bugs with kill actor policy. Will update soon.

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