openwebwork / pg

Problem rendering engine for WeBWorK
http://webwork.maa.org/wiki/Category:Authors
Other
46 stars 76 forks source link

Reducing perl warnings from existing problems #289

Open bldewolf opened 7 years ago

bldewolf commented 7 years ago

We see a lot of perl output into our Apache logs from problem rendering. To reduce this, I created a script that can headlessly render problems and collect their error output, if any. The script is available in: https://github.com/bldewolf/webwork-error-scanner

The scanner runs using checkouts of the webwork repos and only takes an hour or two to "render" every problem.

I've fixed a large amount of issues in these two branches: https://github.com/bldewolf/pg/tree/fix-perl-warnings https://github.com/bldewolf/webwork-open-problem-library/tree/fix-perl-warnings

There are still plenty of problems producing output but I haven't had time to work on it and I don't want the fixes to simply languish on my machine. Should I make pull requests for what I have fixed so far? The OPL branch has at least one very large commit, as I fixed a single class of problem in a lot of files.

Some of the errors are more complicated than simple perl style issues. For example, NUM_CMP is misused in lots of places, with missing or incorrect arguments. Fixing the problems to use NUM_CMP correctly is impractical, but fixing NUM_CMP to behave as it always has but without warnings is also a tricky task.

Anyway, this issue is more or less to make the team aware that this script exists and to ask for feedback on where to go from here.

mgage commented 7 years ago

Hi Brian,

This is very nice. I have a similar standaloneproblemeditor https://github.com/mgage/standaloneProblemRenderer which you might like to compare with yours. Mine is a little more elaborate but I also use it to render batches of problems directly. Perhaps we can create a synthesis of your script and mine. Mine has been sitting around since before the semester started waiting until I had enough time to clean it up. One thing it can do is render the problem, grab the expected answers, then rerender the problem while feeding in the expected answers. This tests whether the problem will accept its own correct answers (not always the case). In principle this could be used to test tolerances and many more unit tests for each pg problem.

Two things to note. This standaloneProblemRenderer is directly coupled to PG and if you run it repeatedly, say on the same problem, you can increase the memory use of the process considerably. This means that PG has a memory leak (no surprise) but it also means that since the web is not involved there are tools to locate that memory leak and fix it. It depends to some extent on the pg problem. Some of the ones involving new MathObjects are particularly bad. -- not all of the links made when a module is created within PG are broken. I'd really like to find that memory leak and plug it.

The second thing is to look at the client sendXMLRPC.pl in webwork2/clients/. This has nearly the same command line API as the standaloneProblemRenderer but renders the problem by shipping it off to a webservice and accepting the result. It is much slower -- but the memory leak is the webserver's problem :-) . Sometimes it's more convenient.

The Opaque server which connects the Moodle quiz with the WeBWorK webservice also shares a lot of similar code with the two editors above. Each of these was developed separately and one of my back burner projects was to refactor things so that they could share the same code.

The Mathbook XML project also uses a similar client to sendXMLRPC to call in to the webservice. I'm not sure how far away that code has forced at the moment.

I'm really glad you are interested in this project. Let me know what you think.

As to your actual question about pull requests for the library. I think you should submit them. They might sit there until after semester exams before someone has a chance to pull them however.

This brings up a larger issue of organizing maintainers for pg, webwork2 and the OPL. It's worked pretty well on an ad hoc basis, but with better organization we can recruit more people to help which would allow some of the usual suspects to take a break. I'll try to get to that. Volunteers can email me below. :-)

Mike Gage, Math Dept University of Rochester Rochester, NY, USA gage@math.rochester.edu

jwj61 commented 7 years ago

For the OPL problems, I think you should just make a pull request. I expect that there will be too many changes for anyone to test them individually, so I would be inclined to just accept it. The only requirement is that it needs to merge cleanly and doesn't accidentally delete any files.

dpvc commented 7 years ago

I looked over the pg changes you made (as that is what affect my code). I'm OK with them, except for one thing: there are some true values that have been set to "true". These should rather be set to 1 instead. Other than that, I'm good with those changes.

bldewolf commented 7 years ago

@mgage Interesting. Mine is more narrowly focused on being essentially a perl -c on for pg problems. Testing a problem's solutions against itself sounds like a great idea for regression testing.

As I wrote my tester, I also became interested in what kind of criss-crossing dependencies between pg and webwork2 existed. It gets pretty annoying. For example, WeBWorK::PG::IO constructs a full WeBWorK::CourseEnvironment just to grab webwork_courses_dir from it.

I also ran into the CPU/memory issues, which is why mine forks before every problem render and sets ulimits on itself. Some problems also get into infinite loops. I've tried debugging tools briefly, but they either take a long time to run, get caught up on the nested evals, or produce more output than I can sort through.

@dpvc I tried to maintain the original behavior while resolving warnings. So, for barewords, I put the string that they would resolve to.

Pull requests created:

290

openwebwork/webwork-open-problem-library/pull/272

dpvc commented 7 years ago

So, for barewords, I put the string that they would resolve to.

Understood, but true was meant to be 1, not "true" (my fault, since I switch a lot between languages, and don't always remember that perl doesn't have true as a boolean literal).

mgage commented 7 years ago

Thanks for looking at my work. One of the by products of building the moodle plugin (opaque_server), the standaloneProblemRenderer and webwork2/clients/sendXMLRPC.pl has been trying to reduce the cross dependencies of PG and webwork2. Considering that PG was originally designed in the late '90s for use with CGI and later joined with the mod_perl front end webwork2 the cross connections are not too bad, and slowly getting better. It's still not as separated as I'd like but perhaps I should push standaloneProblemEditor out to openwebwork so that people with projects such as yours will be aware of it. I'd eventually like standaloneProblemRender to be sufficiently independent so that it could be packaged with PG as a CPAN package and those who simply wanted to edit PG problems without setting up a server or obtaining access to someone else's server could do so.

The criss-cross you mention of building a course environment to get the webwork_courses_dir comes from not wanting to pass those directory addresses through the "environment" variable in PG, which might make it open to detection from the web. Possibly excess caution, but we wanted to get those values from the configuration file. A good percentage of CourseEnvironment is just reading the configuration files and setting values. A great many tools for handling configuration files have been invented since 2002 so perhaps replacing CourseEnvironment with something more light weight, and already in CPAN might help increase the separation of PG and webwork2. It's worth thinking further about. It would probably mean redesigning the configuration files as well, probably to something that looks more JSON. (Would still need to find time to do the work and check that JSON is adequate to replace what is currently done in the configuration files. )

Thanks for processing all those PG problem files !!!