-
Notifications
You must be signed in to change notification settings - Fork 92
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
Fix to get Test and Train Fitness for final solution of Mult-objective Optimization. Fixes #166 #167
base: master
Are you sure you want to change the base?
Conversation
…e Optimization. Fixes PonyGE#166
Thanks for the bug report and PR! This looks good to me, but I have never really used the MOO part of PonyGE. @mikefenton @dvpfagan if still watching this repo, please comment but if not, please let me know and I'll try to test the fix properly. |
Had a review of the codebase there and I think the proposed solution is only treating one specific symptom rather than curing the disease itself. There are a number of problems that should be addressed as part of this fix:
I think the best solution to this problem should be:
Side note after 7 years working in the industry: dear god, we need tests. |
@mikefenton : Thanks for the comprehensive review.
I defined the training_test attribute in the fitness functions as required, but it won't work for multiobjective case because of the moo_ff object being used to store the fitness functions and it does not have direct access to the fitness function's attribute (as currently being checked), yes we can use the fitness functions and then check individual attributes. But even if we define it, the way it works currently is to just check if we have the DATASET_TEST param set then its True otherwise False.
Agreed
A problem needs test dataset is not dependent on how many fitness functions are used. Probably all the fitness functions that are there would be able to have a training fitness and a test fitness. Please correct me if I am wrong here.
Agreed.
This function is only called in the stats.py, line 76 which is for the case of soo, in the moo case this is not called. |
I meant we should just remove the
It's also called in line 109 of file_io in the |
…i-objective optimization PonyGE#166. This was fixed but not pushed in original commit.
Agreed
I had fixed this originally. I was getting the Test, Training fitness in the first front files. Now I have pushed the commit. |
…he example regression and supervised_learning files.
…k as per discussion in PonyGE#130
I'm confused. According to #130 we should change from |
Fix #166