maths / moodle-qtype_stack

Stack question type for Moodle
GNU General Public License v3.0
138 stars 147 forks source link

Second draft of linearalgebra.mac for the contrib folder. #1185

Open LukeLongworth opened 1 month ago

LukeLongworth commented 1 month ago

As before, I don't intend for this pull request to be accepted yet. This is just me publishing the latest version for people to look at.

This is the latest version of #1148 which I will subsequently close. This version marks the point where I've stopped writing new functions (for now) and am now using this file to write my quizzes. I have included two files; linearalgebra.mac which is the file intended for eventual use, and linearalgebra_no_test.mac which is the same, but has no s_test_case lines, as these were causing issues when importing to questions with simp:true. A separate issue, #1184, has been raised concerning this.

The files contain:

Still to-do:

I think I've figured out my workflow now, and will keep everything in this branch, which I think means that I can update this pull request directly? I will figure that out when it comes to it.

sangwinc commented 1 month ago

@LukeLongworth, thanks so much for pushing on with all of this. It looks super useful! I do like the r and c input mechanism. Like the oo etc. input for intervals, it's minimal and reasonable to remember.

We are about to refactor a lot of the Maxima code to include docs within the code, and then automatically generate the user docs from the docs in the code. That puts all the docs into one place. We might, at that point, move the test cases back into the main codebase but strip them out when we include the code within the question. I do really like the principle of having docs, code and tests all adjacent in one file. Perhaps we could have an email discussion about where you are with this? Would you like to include this in the next release and is (part?) of it ready?

sangwinc commented 2 weeks ago

Thanks @LukeLongworth, that's a lot of work. Given it's in the "contrib" folder I'm happy to merge this into master to make it immediately available. I'm happy to do this. Are you ready?

This is a lot of very useful work, thanks.

geoo89 commented 1 week ago

I tested this on 4 different servers (our ecampus, our old sandbox, our new sandbox, and the Uni Trieste server). For 3 of them things run quite smoothly. On our new sandbox, however, the preview doesn't load and then the whole server is unresponsive for a few minutes. Using stack_include with small files works well on that server.

The logs show:

Fatal error: Maximum execution time of 30+2 seconds exceeded (terminated) in /bitnami/moodle/question/type/stack/stack/cas/connector.server.class.php on line 54

At the same time there are also unauthenticated requests to the Maxima service, which stop when cancelling the preview (closing the browser tab).

10.11.2.1 - - [13/Jun/2024:13:42:14 +0000] "POST /versions/2024012900/ HTTP/2.0" 401 172 "-" "-" 77 0.000 [maxima-v2024012900-goemaxima-http] [] - - - - 

For every 401 response from the Maxima service, there is a corresponding timeout message in the Moodle logs. I'm not sure how it's possible for unauthenticated requests to be sent to Maxima when the username and password is set correctly, but it looks like that is what is happening.

It doesn't matter if the CAS connection timeout is set to 10 secs or 100 secs. This server uses goemaxima (like our other sandbox) and is hosted on google cloud platform, but has fewer resources: 'e2-small' (0.5-2 vCPU, 2 GB RAM) as opposed to 'e2-medium' (1-2 vCPU, 4 GB RAM). I don't think this is a stack_include issue because even when pasting the code directly, we get the same issue.


Technically this is a resources/server configuration issue, and we can resolve it by e.g. giving our server more resources.

But it does seem a bit scary that when importing a question from another server and trying to preview it, it could hang the whole server for a while (unless that's also a server setup issue -- seems a bit strange to me that a single request could hang the entire server).

I wonder if it is worthwhile trying to modularize the library a bit to mitigate this risk, so that users don't have to include everything all the time? Is it possible to have recursive stack_include calls?

aharjula commented 1 week ago

Is it possible to have recursive stack_include calls?

No. And in particular, it is impossible to conditionally include stuff.

Having that inside the STACK compiler would be inconvenient, and I (as the one who built this first iteration of the include logic) don't believe it is worth it. Personally, I imagine that whatever is in the "contrib" will eventually end up in the core or become fragmented into small parts easier to include. I also imagine that at some point, someone will decide that they do not like this, and they build some wildly more complicated system based on the fact that the include just includes from an address. We might end up with someone setting up an external package management system which constructs a minimal include with all the required parts based on the URL, e.g.: stack_include(".../include?functions=orthonormal_columnsp,gram_schmidt"), could naturally be that package management system would be part of STACK but it could also be a separate thing but it would require some metadata about requirements etc...

geoo89 commented 1 week ago

I see, that makes a lot of sense.

So with the vision of moving all or a lot of this into core, performance right now is not that relevant in the long run.

stack_include_contrib always takes the latest version from github (according to #971), right? So contributors have to be careful to maintain backward-compatibility, and e.g. splitting up the library once it has been committed is tricky. Until some sort of package management system exists, would it make sense for stack_include_contrib to include an (optional) commit hash or STACK version?

aharjula commented 1 week ago

Yes, it (stack_include_contrib) does indeed take the latest version. The docs also say that these are equivalent, thus its the head of the master branch:

stack_include("https://raw.githubusercontent.com/maths/moodle-qtype_stack/master/stack/maxima/contrib/validators.mac");
stack_include_contrib("validators.mac");

Again, as long as we only work with URLs and have no special extra arguments for the inclusion outside that URL, one can simply tune the URL to get the version one wants. So that those who understand the risks and actually pay attention to these issues can just as well do something like this to get a very specific version of something:

stack_include("https://raw.githubusercontent.com/maths/moodle-qtype_stack/97d70c9ebcff641f204a65091836bd489dc323a7/stack/maxima/contrib/prooflib.mac");

If someone wants to make something like stack_include_contrib("prooflib.mac","97d70c9ebcff641f204a65091836bd489dc323a7") to act as some sort of a shorthand for that feel free but I for one would recommend just writing the full URL when need be. I would not add extra arguments at this point as we really do not know if the future will keep "contrib" where it is now and whether URL construction would be that simple.

LukeLongworth commented 1 week ago

Thanks for your testing of this @geoo89, I don't have the resources to do that myself and it sounds very useful.

I don't think it's a big problem if stack_include_contrib files don't work on all (especially lower-resourced) servers, as I believe that the content should be used with some care anyway. Crashing the whole server when trying to preview a question is concerning though.

I don't mind breaking this into smaller files, but would appreciate some guidance on best practice. My initial thoughts are:

I also agree with @aharjula's comments. I think that this project has stretched the limits of what stack_include_contrib is intended for, and the appropriate response to issues like this is not to change stack_include but to change the inclusions themselves.

@sangwinc I'm not finding myself making many more changes now (i.e. the occasional urge I'm getting to do more with it is more easily quashed) so you're welcome to add it to contrib if you like. As Georg mentioned, it might be more difficult to split up the file into smaller pieces whilst maintaining backwards compatibility.

As a last comment/thought: if there are key functions that I think make sense as core maxima, where would those belong? Their own file alongside intervals.mac and inequalities.mac, or in assessment.mac generally (or somewhere else)?