MuellMark / Course-Scheduler

Repository for the Course Scheduler Capstone Project
http://colbysullivan.pythonanywhere.com/
3 stars 1 forks source link

Investigate Errors from Time Constraints #66

Closed MuellMark closed 7 months ago

MuellMark commented 7 months ago

For some reason, the python code is having the same problems that the glpsol code was having where it is scheduling courses at times should not be possible, as shown here:

Screenshot 2024-03-06 at 9 52 15 AM Screenshot 2024-03-06 at 9 51 05 AM

I'm not sure what's wrong with it yet but I will investigate and make sure to fix it.

MuellMark commented 7 months ago

I've been looking into the code and found a few things. Firstly, I think there is some sort of issue with the faculty times, as having them be forced to a specific time doesn't seem to change the scheduler, so some extra debugging code my be able to give me some more insight.

The constraints are being added and the rows say they are equal to 0, however, the courses are still being scheduled at those times. I'm going to try and make better debugging code that out prints more information to investigate further

MuellMark commented 7 months ago
Screenshot 2024-03-06 at 10 39 40 AM

This showcases the values of some of the rows. Most of the rows behave as normal, but for whatever reason once we get to making sure no 2 courses occur at the same time everything starts getting messed up. As seen above, two different times for CS1 and CS2 are shown to be there, but neither are scheduled at those times

MuellMark commented 7 months ago

I'm going to be picking up where I left off in class and start investigating what is going on. I'm going to start with some very small test cases just to see what ends up happening and going from there

MuellMark commented 7 months ago

My very first bout of tests are using the miniTest.csv file. I'm going to see if I can break the code with only 2 courses and work up from there.

Below are all of the tests, I didn't to make a bunch of tiny files where next to nothing changed, but I do want a record of these

  1. 1st test was just 2 courses: CS1 and DIS, no other restrictions, this obviously had no issues
  2. 2nd test is forcing them both to be 4 contact course and restricting times so that they are set to 8am and 930. This also had no issues
  3. 3rd test is same as second, but forcing them at different times, 2 and 330 (not realistic though) This did not work. There is something wrong with the time constraints that needs to be fixed.
  4. One more test I wanted to do was doing the same thing as 3 but forcing it via a professor, not in the course restrictions. For some reason this actually did what it was supposed to do

I'm not sure as to why the faculty restrictions seem to be working while the course one do not. My guess is that something got off during the construction of the Matrix for the courses specifically. I'm going to do a couple more tests and start comparing the code between the two to see if there is anything majorly different since they should function the exact same way.

MuellMark commented 7 months ago

On my first looks at the code, they seem almost exactly the same with the only difference being the faculty does not have a case for making sure 2 courses don't conflict. Because of this I ran a few other tests:

  1. Same as the 4th one, but I also added in the time constraint of 8 to the courses on top of the faculty. This was a redundant test but it shows that the matrix isn't offset in anyway, at least I think.
  2. The next test I did was having 8 be blocked off by the course requirements and the other two times in the faculty restrictions. Fundamentally, this should work them same as the last 2 tests but it didn't, it scheduled CS1 at 8 despite explicitly saying it is not possible in the CSV.

To me, these tests were very conclusive in showing that there is something wrong specifically with the course time constraints so I'm investigating it further. I'm starting by directly comparing the time requirements code as follows: first is in courses and the other is in faculty: Screenshot 2024-03-08 at 5 53 40 PM Screenshot 2024-03-08 at 5 54 25 PM

When looking at them side by side they go about it in a very different way. The courses go by order of the courses, scanning for the course conflicts or if its a time, then column, then the times are looped through. However, while writing this I think I realized the issue. I'm checking if time==otherCI: but I think otherCI is an int and time is a string. I'm going to fix this, and if this is the case, I want to do a few more tests to make sure nothing else is broken because the two codes that essentially do the same thing are so different.

MuellMark commented 7 months ago

The first thing I did was change the if statement in question to if time==row[otherCI] which kind of worked. No courses were scheduled at 830, but they did break the faculty constraints now. It feels like this may be messing up the matrix but I'm not entirely sure. I'm going to do some more tests to gather more info:

  1. My first test was just moving one time constraint, 930 to the CS1 course from its faculty. This of course just made Dis at 930. I still wanted to check it for my sanity's sake
  2. Next test was moving the other 930, this just moved the next course to 11, which was also specified to not be allowed, which is what I expected but I'm not happy about it
  3. Just to verify, I added 11 to the courses, and now it schedules everything correctly. It's like the problems swapped places with one small edit. This is making me wonder if maybe the issue has to do with how the matrix is constructed, so I'm going to try and move the constraints around in the python
  4. I moved the faculty restrictions up in the python script so they are called before the course constraints which absolutely broke everything even though it shouldn't matter what order the constraints are listed in. Screenshot 2024-03-08 at 6 27 45 PM This what was output, which should not be possible because they are 4 contact hour courses and DIS is listed twice??? This could be an issue with the print method but I think something with the matrix got shifted incorrectly

I feel like I didn't add any new code and I'm going a bit crazy with some of these outputs but I think I made good progress at figuring out what the problem is. I'm going to take a break but when I come back I'm going to see if I can narrow it down any more. The only reason I want to do this is because my alternative is rewriting this block of code (which honestly may be what I need to do) and I want to see if I can fix it without this. Or, if I need to do this, I want to make sure I actually fix it.

I also updated #58 and added one more task to make sure everything is running smoothly with my code

MuellMark commented 7 months ago

I'm going to continue researching this issue. I want to do a bit more testing, as I may be able to discern where the count is getting off. If not, I'll start rewriting code

MuellMark commented 7 months ago

The first thing I did was start getting the lengths of the lp matrix, which only shows the number of rows which was not insightful. I've started looking into the row matrices and it feels more and more confusing because you can assign them as normal, a matrix of ones and zeros, but then they come back like this:

Screenshot 2024-03-10 at 2 23 17 PM

This link has some explanations as to what they mean,

I think it works by doing the row index then column index, which if that's the case I may see the problem. Adding to the matrix may just be adding values, not increasing the actual size of the matrix. I'm going to take a small break to eat but I think I may be on the right track to figuring this out

MuellMark commented 7 months ago

I'm going to continue my work on this and I'm going to see if I can refine how I add rows and columns to the matrix

MuellMark commented 7 months ago

I thought I had found part of the issue but the more I look into it the more confused I get.

Screenshot 2024-03-10 at 3 38 16 PM

This is the output matrix that is created. For this particular problem there should only be 20 columns and 21 rows, which seems to be accurate and all only have values of 1.0. This means that the actual structure of the matrix is correct, but there may be some issue with the values? I'm taking one more sanity break but when I come back I'm going to manually check all the values and go from there. I know no actual progress has been made but I can;t move forward until I know what's wrong, and that's also why I'll be working over spring break to make sure the script is working

MuellMark commented 7 months ago

I'm going to manually check the matrix, either in my notebook or in a google doc.

MuellMark commented 7 months ago

I added a couple of methods to print out the names of the rows and columns for my hand made matrix. I'm about halfway done with it and I'll post it in this thread once I'm done to continue observations

MuellMark commented 7 months ago

I started making it in docs and on paper but it started getting confusing so I switched over to a formatted sheets document which is right here. After creating it and filling it out a bit I realized it would be better to do in sheets so I can color code it which is here. I'm almost done with this and I'll start making observations from there. I'm getting very frustrated with all of this and I'm really hoping that I'll be able see what is broken

MuellMark commented 7 months ago

Adding them in, one thing I noticed is that there are some empty rows, which is not a great sign because there's something messing up in the code. Looking into everything more, it's clear to me that there is something very wrong. Just to verify I'm going add in a few more things to the print statements.

There are some very clear issues though. All of the general constraints don;t have problems but 4 contact hour courses, time based courses, etc are scheduled seemingly at random. I'm starting to compare it with my code to see if there is something obvious off

MuellMark commented 7 months ago

I've taken a lot of notes on things to try and test out, but it will take a lot of print statements to really wrap my head around everything and I'm starting to get burnout trying to figure out what is going on. The matrix I made has been helpful in showing me that at least half of it is working as expected.

I'm going to spend my last half hour on the poster as a mental break. Next week I'm going to do my best to make sure the script is working, even if I go over 5 hours

MuellMark commented 7 months ago

I'm coming back to this issue and I'm going to try my best to figure out what is going on with everything and how I can fix it.

MuellMark commented 7 months ago

I've been adding in a lot of print statements to try and figure out just what the code is doing. I've been looking into all of the matrix calls but I haven;t figured anything out yet. I'll report here as soon as I do.

I don't want to commit these print statements unless they help me figure things out. I may make a separate branch to store these commits, but I don't want the prints crowd the meaningful commits

MuellMark commented 7 months ago

The first thing I'm looking at is the 4 contact hour courses since those were the first indicated that they were incorrect. I have these print statements:

Screenshot 2024-03-16 at 6 35 57 PM

I'm still trying to evaluate them, but I can guarantee that originally the values are correct.

Screenshot 2024-03-16 at 6 40 37 PM

There may be some sort of issue with how they're added? I'm looking further

MuellMark commented 7 months ago
Screenshot 2024-03-16 at 6 53 52 PM

This is a comparison from the two, before and after it's added. It's clear to me that nothing is wrong with this iteration of creating the temp_matricies, but something is off when adding them, as it's added to rows 14,15, and 16, when it should only be the latter 2. I haven't been able to find the cause of it yet, but I am still searching, I think I need to backtrack to the last method

MuellMark commented 7 months ago

The method called before it is the faculty restrictions. I don't see anything glaringly wrong with it, so I started thinking of alternatives. Right now, I make new temp matrices for each row, which get added to a matrix specific to the method, which then gets added to the actual matrix. Somewhere here there is an issue, but the size should be predetermined at the start, but I'm wondering if either A) this is off or B) the trialing 0's are not added throwing everything off, or C) The size isn;t determined in some methods.

One of the potential fixes I have would be to go through and add everything to a seperate matrix list, then at the end assign it to the big lp. Either that, or add in the values in the [row,column,value] order. These would take a considerable amount of work so I'd like to avoid this if possible. I'll do some more digging tomorrow and start working on changes, if required.

MuellMark commented 7 months ago

I'm coming back to this issue today to see if I can find the source, otherwise I'll start figuring out how to rewrite code to fix it

MuellMark commented 7 months ago

I've added various print statements and I'm honing in on the problem I think. I've made a bunch of notes in my notebook that I'll update in a bit, but I have something I need to attend to before I put in the last half of my time. I'll update on this shortly

MuellMark commented 7 months ago

I'm going to pick up where I left off here

MuellMark commented 7 months ago

One thing I noticed was that the faculty constraints were added correctly. As I can see here:

Screenshot 2024-03-17 at 8 27 37 PM

I'm almost 100% sure now that the issue is that the 0's aren;t added at all into the lp.matrix, and if this is the case a good amount of rewriting is in order. I'm checking one more thing, I want to see if anything else points to issues being added, or if it only starts at faculty restrictions

The general_col_cons have no issues since the last value is a 1:

Screenshot 2024-03-17 at 8 37 25 PM

here with the time conflicts we can see than when the matrix has zeroes there arent issues when they are all together as one matrix

Screenshot 2024-03-17 at 8 40 45 PM

I want to check a few more things but I think I know how to fix this now

MuellMark commented 7 months ago

Just as i expected, the courses offered constraints also have a 1 at the end of the list:

Screenshot 2024-03-17 at 9 15 08 PM

I guess I should have explained these, the first list is the matrix that is made through the method, the next is the lp.matrix before and then after said matrix is added.

I'm going to start figuring out the best way to fix this, I have ideas but I want to make sure they minimize repeated code, etc

MuellMark commented 7 months ago

I've evaluated everything and the course of action I'm going to take is adding in a global matrix that I add to in each method which I then add to at the end of the build the lp. This way, no zeroes will be left off.

This should be relatively easy to do, but if this doesn;t work, then I have some back up plans that I'll list out in the next comment

MuellMark commented 7 months ago

I'm creating a new global variable for the matrix, which means instead of adding it to matrix.lp, I'll add it to the new matrix. I'll then have a new method that adds that matrix into the lp.matrix.

If this doesn't end up working, I could add a dummy column to ensure that all of the full temp matrix is always added, but this feels sloppy and prone to errors.

At this point I have added in the new global variable and I'm in the processes of reworking all of the other calls. This is all in the seperate branch in case I break something

MuellMark commented 7 months ago

I got a bit hung up trying to find an efficient way of adding in the variable, as it needed to be called in every method but I didn't want to add it in every method call. Luckily this was fixed by using global, which I had forgotten about for a second since I rarely need to use it. Or so I thought, I just followed what I did for rowIndex and that seemed to work just fine (for now)

I'm starting with a rolling test, just changing in a few of the methods first and if that works I keep expanding it. The first one I did was a method that wasn;t called on in the mini test just to make sure that didn't break anything, and it didn't.

I went ahead and changed all of the methods to include the new global var and from my first look it LOOKs like it's working but I'm very hesitant to say that quite yet. I need to move some print statements around for further testing

Here is the first test:

Screenshot 2024-03-17 at 10 40 59 PM Screenshot 2024-03-17 at 10 41 13 PM

This is great progress because this is valid! I'm wrapping up my 5 hours here but I am going to do more testing in class tomorrow to determine if it is working and next steps accordingly.

MuellMark commented 7 months ago
Screenshot 2024-03-18 at 10 21 52 AM

This is the new matrix, compared to the old one:

Screenshot 2024-03-18 at 10 23 45 AM

These are different matrices, but they have a notible difference. Plus:

Screenshot 2024-03-18 at 10 27 52 AM

This is the correct ouput (for the csv)

MuellMark commented 7 months ago
Screenshot 2024-03-18 at 10 40 15 AM

This is the test schedule that has 14 courses and I dont see anything wrong... yet. I'm going to check a few more things, then clean it up, then push it before I close this issue

MuellMark commented 7 months ago

I spent the class period checking the values manually. One thing I noticed is that there may be something off about the printing, as there are 2 PRL's displayed. Other than that it is working correctly! I'm going to look into the printing before I start cleaning up

MuellMark commented 7 months ago

I added a working fix, where I check if something is a duplicate, and if it is, dont add it... It's not very clean but it works and until it doesn;t work I'll leave it like that. I'm going to merge my branch into main now while I'm here since My branch is very behind and then everything later

MuellMark commented 7 months ago

Everything is merged so I'm going to clean up the code and then close this issue

MuellMark commented 7 months ago

During class time we decided to meet virtually. I decided to take this time to go ahead and clean up all of the code because I had to add a lot of ugly print statements to troubleshoot. That and I wanted to add a bit more documentation to make sure I don't get confused. I'm going to consider this issue resolved and merge my branch to main again just to give everyone access to the cleaned up code. I'm the going to look at #58 and see where to go from there