maths / moodle-qtype_stack

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

Question behaviours, do we really want to use anything else than adaptive mode? #145

Closed aharjula closed 7 years ago

aharjula commented 9 years ago

Hi,

In my own branch I have been doing things that may require a new behaviour class, probably something extending adaptive mode, and while I have been thinking how to handle cases where the behaviour would be something else than the new one or adaptive mode that I can actually work with I have come to the following realisation:

There is no reason why we should even try working with other behaviours. I.e. my branch will soon just give an error message instead of question-text in those cases where it needs to have the correct behaviour active (if the question uses state-variables it needs a behaviour that can support them)...

And when I started thinking like that I started to wonder if normal STACK should also do the same, basically, give an error or warning when used with the wrong behaviours, with maybe a global option to turn that feature off. Reason for this is that I am starting to get annoyed by teachers who have said that they tested the quiz and it looked just fine while the questions actually gave no feedback as they were using the default behaviour whatever that is.

What do you think about this? Do people really use STACK with something else than adaptive mode and if so why, are other question types incompatible or are there other obvious reasons?

-Matti

sangwinc commented 9 years ago

This is a good question Matti,

I only use adaptive mode myself. That said, I know STACK is going to be used for examinations in the near future, so this is one area where you want to be confident that there is no feedback. The behaviours mechanism is very elegant, and I don't want to force a behaviour from the STACK side. I'd be interested if other people use different behaviours. I hope Tim will comment on the OU experience here.

aharjula commented 9 years ago

About not giving feedback:

Funny story, just Monday I got a worried support message that had gone through multiple help desks where a student thought STACK did not work as it only said "Your answer was interpreted as..." every time they clicked check, it sure was odd behaviour especially as the question behaviour was the correct "Adaptive mode" but what was wrong was the "Review Options". The course personnel had turned off "Specific feedback" (and even "Overall feedback") which basically meant that the student got no feedback and could newer know if they got points or not or if the answer was correct, but they still could keep sending answers which still collected penalties. STACK was obviously not broken but IT-people were already calling up a response team to fix this new failed feature...

Anyway we typically leave everything but "General feedback" and "Right answer" on in the "Review options" and give those only after the quiz closes (naturally, different things for training materials). Maybe "Adaptive mode" STACK question should say something if "Specific feedback" is off or maybe this is too much hand holding.

timhunt commented 9 years ago

Deferred feedback is very important to the OU. Here are the behaviours used by all the question attempts in our database:

Bahaviour Question attempts
interactivecountback 777523
dfexplicitvaildate 309616
deferredfeedback 6958
immediatefeedback 2

As Chris says, "The behaviours mechanism is very elegant, and [we should no] force a behaviour from the STACK side." Ditto for forcing particular review options. The flexibility that Moodle provides is good. We should work with it.

Can you explain why you think that you need a new behaviour?

aharjula commented 9 years ago

The need for a new behaviour is related to the state-variable expansion of the STACK-question-type. Basically I need write-access to the question-attempt-step objects in places where the current behaviours just gives me read-only access. Or ideally I need to write to the pendingstep from the question, and I see no way to access that through current interfaces... Hopefully, Chris can show you what I would do with that. Basically, questions that reveal inputs as you progress through various steps and other wacky things.

Currently I have the code working with adaptive mode but in that case I have to do ugly things like writing to read-only steps by accessing the database directly and as I only know about the previous step I can only input state-data to the past, thus making it slightly difficult to analyse the behaviour of the state-machine like question directly using the recorded step-data.

In any case questions utilising state-variables will simply not work with behaviours that are not "adaptive". And that case will have an warning message. Luckily it is easy to see which questions need those.

But obviously there is no need to limit the possibilities of inventive users by denying anything. But I would like to have the option to deny certain things that are not sensible in our environment. Would not be an issue if I just could set the defaults for all the quiz parameters to match the normal STACK use case we have, but there are other users of the Moodle with other things that would also break if used with wrong settings.

timhunt commented 9 years ago

1) the separation between question type and question behaviour is a really important separation of concerns (http://c2.com/cgi/wiki?SeparationOfConcerns) in the whole question system. You may be tempted, in the short term, to trample all over that, but it would be a really bad mistake in the long term.

It may be that understanding the thinking behind that separation of concerns is not properly explained anywhere. However, you must understand it before you propose changing it, and if you have only ever used adaptive behaviour with STACK questions (rather than a range of question types with a range of behaviours) then you won't understand it.

2) Just needing access to save data in the step is not a good reason to make a new behaviour. That is a bad work-around. Moodle tracker issue https://tracker.moodle.org/browse/MDL-30442 exists to request proper support for this in the question engine.

3) However, in every case so far, that has not acutally been necessary to make that change. It has always been possible to do 1 of two things:

4) In addition to the separation of concerns thing. The inability to store state in subsequent steps is a good part of the design in another way. It means that after start_attempt/apply_attempt_state, all the methods of the question class are pure functions without side-effects. The result only depends on the initialised question object and the data passed into the method. This is a really good thing for avoiding subtle bugs. Therefore, I am very strongly tempted to close MDL-30442 as won't fix.

Having said all that, can I reassure you that I know that what you are trying to do with question blocks etc is really good educationally, and Moodle should allow things like that to be implemented. However, the way to implement it must be in keeping with the existing desing of the Moodle question system.

I don't yet understand how your code works at the moment, so I can't really say more about the right way to do things. Sorry.

aharjula commented 9 years ago

Ok. So I did not know about MDL-30442 and it has something similar to what I need but not quite. As to 3:

As a short description of what the state-variable thing does:

So if there is no way of doing this without breaking that separation this leaves me two options:

  1. Build my own key-value store with its own cleaning (remove attempts remove keys) logic and usageid-like keys, which would be a shame as there is such a perfectly fitting ready made thing there already.
  2. Find a way to generate inputs that are magically force submitted so that I can use them to write to the step-data.

In any case neither of these ways enables the state-variable using questions to work with behaviours that do not allow multiple answers to the same question. Those questions just could newer get beyond scene one of their storyboard with those behaviours.

timhunt commented 9 years ago

A) "what the question displays depends on what the student does"

This is already true in STACK. Think about the validation. Or that the feedback depends on the answer the student gave.

B) "questions internal state" - questions should have any changes in iternal state, after they have been started.

Note that, the current state of the question is comletely determined by

It is completely deterministic. You may worry that re-computing it each time will be too slow. However, that is just a performance issue, and you should not try to optimised it until you konw whether or not it is a real problem. "Premature optimization is the root of all evil." http://c2.com/cgi/wiki?PrematureOptimization If there is a performance problem here, think caching - like the CAS cache which I mentioned before.

C) "a way to generate inputs that are magically force submitted" - no magic is required. You can use for this. That is how the validation works.

timhunt commented 9 years ago

Just one other note: It may still be the case that a new behaviour is neede for this. But, if so, it should only be used for questions that really need it. If you look at the qtype_stack_question::make_behaviour() function (https://github.com/maths/moodle-qtype_stack/blob/master/question.php#L215) then it already looks at various properties of the question, and the behaviour that the quiz is requesting, in order to select the right behaviour.

So, it might be appropriate to say there if (this question uses the new question blocks code in a non-trivial way) then use some new behaviour, say qbehaviour_fullyadaptive.

However, we should not change the behaviour of any existing questions that currently work the way the user expects.

aharjula commented 9 years ago

Ok so maybe I am not coming clear on the level of complexity of the needed evaluation to regenerate the state if there are no way to store the state.

A) Well in this case the questions behaviour depends on all the previous inputs of the student as well as the current ones.

B) The state of this type of expanded question is something the question author defines and can modify depending on the users inputs. If I were to re-evaluate it on each step I would need to do it like this:

  1. Load the initial state generated during the initialisation which I can save.
  2. Go through each and every attempt-step for inputs that could activate PRTs and feed them through all the relevant PRTs in order to generate all the changes to the state, each of these steps doing a CASSession evaluation, luckily its cached...
  3. That is what I have to do before I even display the question or redraw it.
  4. Then comes the inputs and as I have no memory I have to go through step 2 again to find out the current state of the question so that the question knows what to do with the input.

Now consider that situation with a question that asks for something and based on the answer asks for something else and then continues the process onwards, revealing new inputs on the way, in my vision these would typically go through three or four such steps with possibly some branching if something needs to be clarified. The problem is that if everything needs to be built from scratch every step of the way the response time grows every step and I do not believe that the speed it grows in in any way insignificant, especially as all those evaluations go through Maxima or CASCache.

C) "You can use ???? for this." So are you saying that I could generate input values for fields that have not been defined in advance inside grade_response(array $response) and those would get stored to the current pending step, and it would not break any design rule? As In my way I would have to render hidden inputs to the question text and somehow force them to move the last stored state step forward.

timhunt commented 9 years ago

C) sorry, I forgot that markdown does not excape HTML It should say: You can use <input type="hidden">

I think it would be really helpful if you could give one or two examples of questions that will use these new faciltiies. Simple enough to be easy to understand, but with enough complexity to show all the issues that need to be considered.

aharjula commented 9 years ago

Better to give you an example of a question code. That works, but naturally, you would need to have my branch to use it, but I believe you can see what it does from just the code.

timhunt commented 9 years ago

Sorry, I don't have time to install and learn to use your branch now, and that link to the sample question does not seem to work.

That is why I say it would be helpful if you could describe an example or two in words, or record a screen-cast, or something.

aharjula commented 9 years ago

Doesn't work? No XML downloading?

Well it is a question that presents a polynomial (with unique roots) and asks for the student to give one of that polynomials roots. When it gets a correct root if steps forward and divides that root out of the polynomial then asking for the next root for the remainder of that polynomial. During the second step it gives specific feedback if the student tries to give the same root again (which is possible even though each input is tested in separate PRTs as the test has access to the state-variable storing already found roots), this continues in this example until three roots of a fourth degree polynomial have been found. The student may input those roots in any order they wish and the question text presents the remainder is based on the students inputs. The question is implemented so that the student may only give one root at a time and can't go forward before a correct root has been given, in addition to that the previously given roots can't be changed as the input boxes those were given into have disappeared.

The state of the question defines the questions presentation as in what is the remaining polynomial and what roots have been found and which of the input fields should be displayed. The displaying is done with the if-blocks which check for values of state-variables.

The state changes only in the PRT feedback variables and in this case only if we are finding a new and correct root.

Oh and just for fun that question says "Well done $firstname" at the end of the question... as why should a state-variable using question be limited to its internal state, why not do string operations with peoples names in Maxima.

sangwinc commented 9 years ago

I am following this discussion!

Tim, I've learned a huge amount from you over the years ("state is evil" is just one mantra I've absorbed) and you've nudged me in the direction of avoiding "premature optimisation" on a number of occasions with very good outcomes.

Matti, perhaps you could put your question interactions into a PHP unit test, with an explanation of each step? Of course, at this stage it doesn't have to pass!! This would serve many purposes and document our intentions with the code. When we spoke a couple of weeks ago I could envisage some other uses of state. I do like the $firstname feedback, but for me this is a "nice to have" rather than an essential feature. (Some educational research might tell us to personalise in this way, we shall see!)

We may not need behaviours, and I honestly don't know. But first we should try to explain what we do need in terms of how we expect the system to behave at a top level. Your colleagues in the abacus project will be able to help with real questions and interaction use cases which we can't currently accommodate.

I have to say, I came away from Finland quite excited at the potential of what you have done. I also came away aware that there is a long way to go in making other people understand this! I've always worked from real educational examples, and I'd like to try this here.

C

aharjula commented 9 years ago

Well it seems the original question in this issue has now gotten its answer, no warnings for odd usage combinations should be given as it would not be proper behaviour.

It seems like I'll just have to go with qtype_stack_question::make_behaviour() to ensure that questions that happen to use state-variables to store things (they could just be reading things during initialisation which is a different thing) won't get behaviours with only one "attempt".

As to those unit tests, they are coming but it will still take time, probably the example in those is the polynomial question as it has obvious "scenes" with a simple acyclic scene-graph and it's correct answers depend on previous inputs to other input fields. Currently I am finalising the edit_forms validation that all references to state-variables have been declared.

As to "state is evil" it has its merits but I am a bit less black and white kind of a person and would like to note that while it is always possible to rebuild everything from those (if it is time-invariant or if we can fake the time to match the input-logs, actually there has been a plan to have stack_state_get("system","unixtimestamp",0) available) there must be a point where "caching" something as a "state" would become sensible due to the re-generation cost rising. Same to "premature optimisation" if experience says that the costs of a certain path are too high do we really need to go that way?

Hopefully final question about the off-topic sate-variables and the last wall of text from me about this, the options I see now are: A) a new behaviour, that allows the question to write to step-data and more importantly to the $pending_step if at all possible. B) build a new key-value store with similar history management (we still need re-play to give teachers a way to see the students process) as step-data, I would just need to find a key for it to identify this use of this question i.e. usageid but I can't see that in the question... C) build a state regeneration code that replays everything from input values starting from the initialisation moments base state D) same as C but with moving the base forward by writing things through hidden inputs injected to the question text (and even then that output must be submitted to be stored) that will be highly unreliable way to move the base point of the regeneration forward. In this solution I will also need some cryptography as the state is internal to the question and could contain things that the student should not know and as I would need to pass it through the browser... the only benefit of this and Cs is that as I do not store the state anywhere I do not need to limit the length of variable names as they do not need to be limited by real things like key-value stores and instead of storing values using separate keys I would just use one key to store encrypted blob of them all, but the price of that one benefit is that reporting can't easily filter by variable values without unpacking that and if it encrypted with a key tied to the Moodle installation you can't export those attempts to another Moodle installation. E) Continue using the current hack that writes to the previous attempt_steps data by accessing the database directly, thus making playback/reporting of the students actions difficult if not impossible (breaks at the first step)

In my view A is the way to go B is bad if not even stupid duplication of a thing that exists, C is useless step to prove that a thing that I know to be slow is slow and D is just an ugly hack to try to fix Cs performance issues. And while E works it breaks my view of valid coding standards so badly that I can't live with it. Also only options A and B seem to be such that they do not impose obvious limits to the future development of the question type or at least the questions of this type that use state-variables.

So Tim do you agree in my views of those five options? Or do you already know of an better option?

aharjula commented 9 years ago

There are now tests describing that state-variable question. Naturally, those tests do not work due to question_state_invalid Object which is probably appearing because the testing environment is not the same as a real session with real attempts, that can be easily hacked like in the solution E.

aharjula commented 8 years ago

Ok, time for a report on those options A-E:

A) Could still be the way to go. Probably better not to actually create a new behaviour, instead use the access given by qtype_stack_question::make_behaviour() to gain access to the behaviour and work through that even when it breaks some walls in the question model.

B) Would need to hook up to the cleaning process, but how. I now know that I can get the usage id and slot from the question_attempt passed to qtype_stack_question::make_behaviour() from public functions that do not even forbid it in their descriptions no less... so this is the cleanest solution from the question models point of view.

C) Requires too much, just to rebuild the requests for each step was a complication, basically required replay of the question through regrade like logic and that most certainly was not fast. Requires breaking the walls built between question_attempts, steps, behaviours and question types. Tried to do this by rebuilding the question for the steps in order, the number of pointless cassessions was staggering (small fraction of them could be cut out if one were willing to duplicate the processing logic but that would be senseless) and response times grew to five seconds range after couple of steps and as this type of a question can easily generate a dozen or more steps with all the validations, adding multiple questions of this type to the same quiz even on different pages grew the problem even further.

D) Does not work, cuts down the number of replays needed to just the previous step (if we hack through various little walls in the code and can write the exit state after PRT evaluation) or two previous steps (if we only write the entry state) but as we can't trust the state coming in we need crypto and evaluation from previous states even if we were to get the exit state of the previous request.

E) Not really an option, but can be cleaned greatly even the playback issues can be solved by defining and storing entry and exit states separately.

So after months of trying to think of a way to solve this it is apparent that Moodles question model does not support this (this being the storage of the exit-state as well as entry-state, this might fit it if we assume that we do not care about response times and can always start from scratch but even then we need to open up some walls to gain access to the previous steps/requests). But as I want this I now have to choose a way to do this in a way that causes the least amount of problems and it seems to be the model B mixed with forcing the correct behaviour as it requires no walls of the question model to be broken. If no hook can be added to the cleaning of question_attempts we will need to add additional things to Moodles cron but that should not be a major issue.

So one more table in the database, hopefully no one will blame me for adding one when there was perfectly similar table already there, but maybe this little wall of text explains why I have to add to that set of tables that is Moodle.

Still from general question development point of view I do still wonder why the step_data is not writeable after initialisation. And I most certainly do not understand this comment what does the processing there mean, is just the initialisation phase considered processing I for one would think input was something that you process?

aharjula commented 7 years ago

Cleaning up the issues list.

This thing will happen in a new question type which will have its own behaviour so not an issue for STACK.