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

Refactor graphs, streamline some polyhedral constructors #2952

Merged
merged 6 commits into from
Dec 6, 2023
Merged

Conversation

lkastner
Copy link
Member

@lkastner lkastner commented Oct 24, 2023

  • Reduce duplication
  • Let {add,rem}_edge! return bools whether operation succeeded.
  • polyhedral_fan and normal_toric_variety now use the same order of arguments as polyhedral_complex. @YueRen
  • Due to Matrix{Int} being used for both encoding incidences or rays in normal_toric_variety it was impossible to cleanly deprecate all old constructors.
  • Rename pm_tdivisor to pm_object as this is used everywhere else.

@lgoettgens
Copy link
Member

lgoettgens commented Oct 24, 2023

  • Let {add,rem}_edge! return bools whether operation succeeded.

Are there situations where this does not succeed? If yes, that would imply that I need to check the return value every time to be sure, right?

@lkastner
Copy link
Member Author

  • Let {add,rem}_edge! return bools whether operation succeeded.

Are there situations where this does not succeed? If yes, that would imply that I need to check the return value every time to be sure, right?

yes

@lkastner lkastner force-pushed the lk/graphs branch 2 times, most recently from 579931f to c7a1546 Compare December 6, 2023 08:48
@lkastner lkastner changed the title Minor refactor of some part of graphs Refactor graphs, streamline some polyhedral constructors Dec 6, 2023
@YueRen
Copy link
Member

YueRen commented Dec 6, 2023

Ah, I see that you already changed groebner_fan for me, much appreciated!

@lkastner lkastner marked this pull request as ready for review December 6, 2023 09:42
src/Combinatorics/Graphs/functions.jl Outdated Show resolved Hide resolved
Copy link

codecov bot commented Dec 6, 2023

Codecov Report

Merging #2952 (a621b88) into master (1f2b71b) will increase coverage by 25.34%.
Report is 4 commits behind head on master.
The diff coverage is 87.71%.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2952       +/-   ##
===========================================
+ Coverage   55.15%   80.49%   +25.34%     
===========================================
  Files         511      521       +10     
  Lines       69234    70405     +1171     
===========================================
+ Hits        38185    56676    +18491     
+ Misses      31049    13729    -17320     
Files Coverage Δ
experimental/FTheoryTools/src/auxiliary.jl 93.50% <100.00%> (+23.80%) ⬆️
...ricVarieties/AlgebraicCycles/special_attributes.jl 100.00% <ø> (ø)
...ometry/ToricVarieties/CohomologyClasses/methods.jl 60.00% <ø> (+4.00%) ⬆️
...ies/NormalToricVarieties/standard_constructions.jl 99.24% <100.00%> (+0.75%) ⬆️
...ebraicGeometry/ToricVarieties/Proj/constructors.jl 92.72% <100.00%> (ø)
...eometry/ToricVarieties/ToricDivisors/attributes.jl 100.00% <100.00%> (ø)
...metry/ToricVarieties/ToricDivisors/constructors.jl 89.79% <100.00%> (+12.24%) ⬆️
...eometry/ToricVarieties/ToricDivisors/properties.jl 78.57% <100.00%> (ø)
...Varieties/ToricMorphisms/standard_constructions.jl 100.00% <100.00%> (ø)
src/Combinatorics/Graphs/functions.jl 93.45% <100.00%> (+4.37%) ⬆️
... and 6 more

... and 321 files with indirect coverage changes

@lkastner lkastner merged commit ee272fe into master Dec 6, 2023
19 of 23 checks passed
@lkastner lkastner deleted the lk/graphs branch December 6, 2023 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants