Opened 5 months ago
Closed 4 months ago
#32014 closed enhancement (fixed)
Add GaussLegendre vector integration with specified number of nodes
Reported by:  ghDisneyHogg  Owned by:  ghDisneyHogg 

Priority:  major  Milestone:  sage9.4 
Component:  numerical  Keywords:  
Cc:  nbruin, slelievre  Merged in:  
Authors:  Linden DisneyHogg  Reviewers:  Samuel Lelièvre, Nils Bruin 
Report Upstream:  N/A  Work issues:  
Branch:  11feeb2 (Commits, GitHub, GitLab)  Commit:  11feeb2ee1a3841a3eaaca42f0acb2dbd860eeaa 
Dependencies:  Stopgaps: 
Description (last modified by )
From #30698.
We add a method integrate_vector_N
.
Change History (16)
comment:1 Changed 5 months ago by
 Cc nbruin slelievre added
 Component changed from PLEASE CHANGE to numerical
 Owner changed from (none) to ghDisneyHogg
 Summary changed from functionality_of_integrate_vector to functionality of integrate_vector
 Type changed from PLEASE CHANGE to enhancement
comment:2 Changed 5 months ago by
 Branch set to u/ghDisneyHogg/functionality_of_integrate_vector
comment:3 followup: ↓ 4 Changed 5 months ago by
 Commit set to b1d61ba69958bbdba7448193609a1def428d669a
comment:4 in reply to: ↑ 3 Changed 5 months ago by
Replying to ghDisneyHogg:
Because a version of this ticket was started as part of #30698, this ticket should eventually be merged with that ticket, to resolve the conflict.
One way of dealing with these kinds of merge conflicts is to set the "Dependencies" field to #30698, and rebase this branch on the branch there. Once the tickets are ready to be merged, this clearly indicates the merge order. It may mean that the branch here must be merged with further developments on the other branch. It's actually a practical reason *not* to split tickets you're working on, if they are mainly touching the same file.
You can also hope that git's standard merge tools recognize you're touching different lines in the same file and hence merge without reporting conflicts (logically, the changes on the tickets are orthogonal, so the dependence would really be for practical reasons)
comment:5 Changed 5 months ago by
 Status changed from new to needs_review
comment:6 Changed 5 months ago by
 Description modified (diff)
 Reviewers set to Samuel Lelièvre
 Summary changed from functionality of integrate_vector to Add GaussLegendre vector integration with specified number of nodes
Can you fix the typo "incomatible" for "incompatible" and maybe rephrase
 It is different from ``integrate_vector`` by fixing the number  of nodes to use, rather than aiming for a certain error. + It differs from ``integrate_vector`` by using a specified number + of nodes rather than targeting a specified error bound on the result.
and remove the extra blank line at the end of the docstring?
comment:7 followup: ↓ 8 Changed 5 months ago by
I think a better interface would be to just extend integrate_vector
so that one can either use epsilon
or the number of nodes as a keyword argument.
comment:8 in reply to: ↑ 7 Changed 5 months ago by
 Description modified (diff)
comment:9 Changed 5 months ago by
 Commit changed from b1d61ba69958bbdba7448193609a1def428d669a to 7fc196b04f5708380b49195a4f2c0b1bcb07d396
Branch pushed to git repo; I updated commit sha1. New commits:
7fc196b  Typo fix and rephrasing.

comment:10 followup: ↓ 11 Changed 5 months ago by
 Status changed from needs_review to needs_work
Merge fails; branch needs rebasing.
comment:11 in reply to: ↑ 10 ; followup: ↓ 12 Changed 5 months ago by
comment:12 in reply to: ↑ 11 Changed 5 months ago by
Replying to ghDisneyHogg:
Replying to nbruin:
Merge fails; branch needs rebasing.
Can you give more detail please? Were you merging #30698 into this ticket and it had a conflict?
If you look at the ticket description, you can see that the branch is displayed in red, and if you hover over it, you see "trac's automerging failed". If plain merging by git gives no problem, this indicates that trac tries a merge with particularly conservative and sage settings. However, I think you'll at least have to make sure that trac can do the merge, because otherwise reviewers have a harder time seeing what's happening on the ticket.
(I suspect the problem is that the current branch makes changes to an ancestor of "develop" that end up identical to what eventually happens in "develop", but are proposed separately. It would for instance mean that the "blame" of the relevant lines would be ambiguous. While in university administration and politics, this would be a valuable feature, it is slightly undesirable for the modification history of a software project)
comment:13 Changed 4 months ago by
 Commit changed from 7fc196b04f5708380b49195a4f2c0b1bcb07d396 to 11feeb2ee1a3841a3eaaca42f0acb2dbd860eeaa
comment:14 Changed 4 months ago by
 Status changed from needs_work to needs_review
Merge fail has been fixed.
comment:15 Changed 4 months ago by
 Reviewers changed from Samuel Lelièvre to Samuel Lelièvre, Nils Bruin
 Status changed from needs_review to positive_review
Documentation has been adjusted, routine itself is basically trivial (but necessary for upcoming tickets), so: positive review!
comment:16 Changed 4 months ago by
 Branch changed from u/ghDisneyHogg/functionality_of_integrate_vector to 11feeb2ee1a3841a3eaaca42f0acb2dbd860eeaa
 Resolution set to fixed
 Status changed from positive_review to closed
As per ticket #30698, move the updates to sage.numerical.gauss_lengdre.integrate_vector, namely allowing for the specification of the number of nodes desired a priori, to a new ticket.