small NormalToricVarieties fix to retain CoefficientRing and Variable #3442
Conversation
Devlin-Mallory
changed the title
| ); | ||
| coneList' = for t in clStar list ( | ||
| if all (s, i -> member (i,t)) then continue | ||
| else t | {n} | ||
| ); | ||
| Z := normalToricVariety (rays X | {v}, coneList | coneList'); | ||
| Z := normalToricVariety (rays X | {v}, coneList | coneList', CoefficientRing => X.cache.CoefficientRing, Variable => X.cache.Variable); | ||
| Z.cache.toricBlowup = X; |
There was a problem hiding this comment.
Do you know why the base X is cached here but not above?
There was a problem hiding this comment.
Not sure I understand the question - where above would/should X be cached?
There was a problem hiding this comment.
Like 5 lines above, the other case where a toric variety is constructed and returned.
There was a problem hiding this comment.
Oh, I see now - I'm not sure why that differs. I could add the base into the cache, unless @ggsmith or anyone else has objections?
There was a problem hiding this comment.
For consistency, I think that the base toric variety should also be cached in the #s === 1 case. @Devlin-Mallory can you also make this change?
There was a problem hiding this comment.
These changes look good to me.
| ); | ||
| coneList' = for t in clStar list ( | ||
| if all (s, i -> member (i,t)) then continue | ||
| else t | {n} | ||
| ); | ||
| Z := normalToricVariety (rays X | {v}, coneList | coneList'); | ||
| Z := normalToricVariety (rays X | {v}, coneList | coneList', CoefficientRing => X.cache.CoefficientRing, Variable => X.cache.Variable); | ||
| Z.cache.toricBlowup = X; |
There was a problem hiding this comment.
These changes look good to me.
|
Thanks for the fixes, and the review :) |
Several methods in NormalToricVarieties (notably toricBlowup and thus makeSmooth and makeSimplicial) were discarding the CoefficientRing and Variable of the input toric variety. This fixes that by retaining these options from the input toric variety.