Skip to content
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

checkParams() should run on defaults once sequence is chosen #401

Closed
katestange opened this issue Jul 24, 2024 · 7 comments
Closed

checkParams() should run on defaults once sequence is chosen #401

katestange opened this issue Jul 24, 2024 · 7 comments
Labels
bug Something isn't working question Further information is requested ui Something having to do with the user interface

Comments

@katestange
Copy link
Member

The default values for visualizer parameters may not be valid in the context of a particular sequence. This could be checked by checkParams(), but it appears that right now checkParams() is not run until the user touches the parameters. For example, if a turtle visualizer or a difference visualizer wants to use the first N terms by defalut, but the sequence at hand has fewer than N terms (there are some sequences with very few terms in the OEIS!), such a check may fail. If it isn't checked, an error is thrown later on in the code when we try to fetch too many terms.

@katestange katestange added bug Something isn't working ui Something having to do with the user interface labels Jul 24, 2024
@gwhitney
Copy link
Collaborator

Hmmm, I am confused. Scope.vue creates a visualization by calling Specimen.fromQuery. That calls validate() on first the sequence and then the visualizer. And validate() in turn calls checkParameters(). So I don't see how a validation can ever get on the screen without going through checkParameters(). Can you provide an explicit recipe for something to go wrong and an explanation of what exactly goes wrong? I tried Differences with all defaults (wants 20 terms) on A371958 (only has 15 terms) and all seemed to go fine. Admittedly, Differences tolerates its parameter called "n" being larger than the number of entries in the sequence and just deals with it in draw(), so it doesn't seem like anything can really go "wrong" there, but maybe the issue you are identifying can cause real problems in another context?

@gwhitney gwhitney added the question Further information is requested label Jul 29, 2024
@katestange
Copy link
Member Author

Ok, well, I noticed this while working on the turtle-enhance branch, so it's always possible it's actually something in that branch. For now, here's how to reproduce:

  1. On turtle-enhance branch
  2. Go to http://localhost:5173/?name=errordem&viz=Turtle&pathLength=10000&growth=1&seq=OEIS+A156660
  3. Observe that this has 10000 terms (which the sequence allows)
  4. change OEIS sequence to A371606 (for me I've used this sequence before, so it's in the sequence chooser)
  5. now the turtle ends after 16 steps, but it shows that it is using 10000 terms. If you put the mouse in the 10000 terms input box and try to type, the box will auto-correct to 16.

My essential observation is that it doesn't auto-correct to 16 upon loading the sequence, only upon trying to update that box. I'll try to look into this more when I have time.

@gwhitney
Copy link
Collaborator

Will try to debug when I get the chance, but sounds like it could possibly be a missing refreshParams() call in one of the places the number of terms is being changed. Basically any time a visualizer changes a parameter itself it should immediately call refreshParams().

@gwhitney
Copy link
Collaborator

gwhitney commented Aug 2, 2024

I am thinking this will be fixed by the fix for #403; we shall have to see after that lands.

@gwhitney
Copy link
Collaborator

@katestange same comments as for #403. Thanks for looking into the current status of this.

@katestange
Copy link
Member Author

So we can play with this issue in Chaos, which has a field for the number of terms.

  1. Load http://localhost:5173/?name=OEIS+A071642&viz=Chaos&first=1&last=3605&seq=OEIS+A071642
  2. Switch sequence to A371958 (which has only 15 terms)
  3. Observe that the field for number of terms has gone down to 15, which is good, but the URL still says 3605.

So the behaviour has changed in ui2, but is still kind of weird. In many overhauled visualizers, they'll just use as many terms as available by default, so it won't even be in the url -- like for FactorFence. In some, we may want to limit the number of terms artificially, in which case this issue may (?) return/remain. Which brings up the question of the best way to handle such a situation (Turtle Enhance might want to limit the number of terms below the full number available, for example).

@gwhitney
Copy link
Collaborator

With the current handling of sequence bounds in ui2, none of these things are reproducible. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested ui Something having to do with the user interface
Projects
None yet
Development

No branches or pull requests

2 participants