Opened 4 years ago

Closed 3 years ago

# divided_difference in SchubertPolynomialRing should not throw errors on unused variables

Reported by: Owned by: darij major sage-8.1 combinatorics schubert, polynomials, divided differences tscrim, sage-combinat Darij Grinberg Travis Scrimshaw N/A 37b4648 (Commits) 37b46486872755c05a2c096ac5423b3cc1016786 #23403

### Description

```sage: X = SchubertPolynomialRing(ZZ)
sage: a = X([1,2,3,4,5])
sage: a.divided_difference(4)
```

This should yield `0`, not an error. Elements of X are polynomials in infinitely many variables; when they don't use a variable, it doesn't mean that this variable cannot be divided-differenced over.

Followup to #23403.

### comment:1 Changed 4 years ago by darij

Now solved in #23403.

### comment:2 Changed 4 years ago by tscrim

It is an orthogonal issue from #23403, and this would be a great place to discuss this as there could be a good use case for considering having a finite number of variables and having this throw an error. I agree with the change, but it is better documented by having a separate ticket for it.

### comment:3 Changed 4 years ago by darij

• Branch set to public/combinat/improve_schubert_divided_difference
• Commit set to f915925252394a15968678505299eaf256bb1d06
• Dependencies set to #23403

As discussed with Travis, this ticket is now the place for branch public/combinat/improve_schubert_divided_difference , which builds on top of #23403. Will need rebasing, though, once #23403 is p_r'd.

### comment:4 Changed 4 years ago by tscrim

Okay, I did some more investigating. The biggest time sink in the symmetrica version is not symmetrica, but instead most of the time is spent on checking the input via `reduced_word`. Even with removing that check, enough time is spent on the overhead that even the non-optimized native algorithm beats going to/from symmetrica unless maybe I really spent some time optimizing the interface, which, IMO, is time not well spent.

### comment:5 Changed 3 years ago by darij

• Status changed from new to needs_review

### comment:6 Changed 3 years ago by darij

Turns out no rebase is needed.

### comment:7 Changed 3 years ago by tscrim

We should change `newm` to `algorithm`, which takes `'sage'` or `'symmetrica'`. I also think a better check is `if i in ZZ:` too, but that isn't something I will quibble over. Once that is done (with corresponding documentation updates), this will be a positive review.

### comment:8 follow-up: ↓ 9 Changed 3 years ago by darij

Is that something I should do, or you want to do?

I'm fine with `if i in ZZ` instead of `if isinstance(i, (Integer, int))`, if this is what you mean.

### comment:9 in reply to: ↑ 8 Changed 3 years ago by tscrim

Is that something I should do, or you want to do?

I can do it, but it might take me a little longer to push right now. However, it was somewhat a question of seeing if you agree; granted, I didn't phrase it like I should have.

I'm fine with `if i in ZZ` instead of `if isinstance(i, (Integer, int))`, if this is what you mean.

Yes, it is. The rationale: if by chance a user is doing 2/2, it will still work. Granted, this is not likely to happen, but I've had it happen to me enough that I'm paranoid about it now.

### comment:10 Changed 3 years ago by git

• Commit changed from f915925252394a15968678505299eaf256bb1d06 to 37b46486872755c05a2c096ac5423b3cc1016786

Branch pushed to git repo; I updated commit sha1. New commits:

 ​a5b2cff `Merge branch 'public/combinat/improve_schubert_divided_difference' of git://trac.sagemath.org/sage into schub` ​37b4648 `changes suggested by Travis`

### comment:11 Changed 3 years ago by darij

Done those changes. But be warned that I haven't tested the correct behavior of the 'symmetrica' algorithm on polynomials with too few variables.

### comment:12 Changed 3 years ago by tscrim

• Authors set to Darij Grinberg
• Reviewers set to Travis Scrimshaw
• Status changed from needs_review to positive_review

Thank you.

### comment:13 Changed 3 years ago by vbraun

• Branch changed from public/combinat/improve_schubert_divided_difference to 37b46486872755c05a2c096ac5423b3cc1016786
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.