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

Remove number / vector #44358

Merged
merged 3 commits into from
Mar 12, 2022
Merged

Conversation

antoine-levitt
Copy link
Contributor

Split off from #40758. This change is more straightforward than removing all the pinvs, and unlikely to break user code (the only cases found by PkgEval last time were bugs where number ./ vector was meant).

@ViralBShah
Copy link
Member

@antoine-levitt Should we merge? Good to get this in early into the 1.9 cycle.

@antoine-levitt
Copy link
Contributor Author

I think that'd be good, if you're fine with the breakage (unlikely, and probably of buggy code anyway). Does this need a NEWS item?

@fredrikekre
Copy link
Member

@nanosoldier runtests(ALL, vs = ":master")

@ViralBShah
Copy link
Member

Yes we should do NEWS.

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

@antoine-levitt
Copy link
Contributor Author

There's a lot of packages failing with segfaults, so either PkgEval or master has an issue. From a quick wget -r + grep it doesn't look like any are failing because of this change though.

@martinholters
Copy link
Member

I'm cool with this, but we should eventually (for 2.0) make up our mind what / should mean. Is x/y equivalent to x*inv(y) or x*pinv(y)? Currently the answer is "it depends". (And this PR changes "depends on what exactly?" in a niche case.) We should either come up with clear rules what it depends on or (my preferred route) consistently pick one of the meanings. And I'd then prefer the inv version and come up with another way to write the pinv version. (And all the same applies to \, of course).

@antoine-levitt
Copy link
Contributor Author

Let's revisit #28827, I'll post there

@martinholters
Copy link
Member

Ah sorry, I had forgotten I had even opened a dedicated issue about that.

@ViralBShah
Copy link
Member

ViralBShah commented Mar 1, 2022

@antoine-levitt We'll need a NEWS item about this.

Let's plan to merge this tomorrow, unless someone brings up new concerns on this PR.

@ViralBShah ViralBShah added the needs news A NEWS entry is required for this change label Mar 1, 2022
@antoine-levitt
Copy link
Contributor Author

Done, thanks for reminding me

@ViralBShah ViralBShah removed the needs news A NEWS entry is required for this change label Mar 1, 2022
@dkarrasch dkarrasch added the domain:linear algebra Linear algebra label Mar 1, 2022
@ViralBShah ViralBShah merged commit 24d5268 into JuliaLang:master Mar 12, 2022
@3f6a
Copy link

3f6a commented May 19, 2023

Isn't this a breaking change?

@oscardssmith
Copy link
Member

yes. It's a breaking change that pkgeval found the only uses of were typos.

KristofferC pushed a commit that referenced this pull request Aug 10, 2023
@brenhinkeller brenhinkeller added the kind:breaking This change will break code label Aug 17, 2023
KristofferC added a commit that referenced this pull request Aug 18, 2023
Backported PRs:
- [x] #47782 <!-- Generalize Bool parse method to AbstractString -->
- [x] #48634 <!-- Remove unused "deps" mechanism in internal sorting
keywords [NFC] -->
- [x] #49931 <!-- Lock finalizers' lists at exit -->
- [x] #50064 <!-- Fix numbered prompt with input only with comment -->
- [x] #50474 <!-- docs: Fix a `!!! note` which was miscapitalized -->
- [x] #50516 <!-- Fix visibility of assert on GCC12/13 -->
- [x] #50635 <!-- `versioninfo()`: include build info and unofficial
warning -->
- [x] #49915 <!-- Revert "Remove number / vector (#44358)" -->
- [x] #50781 <!-- fix `bit_map!` with aliasing -->
- [x] #50845 <!-- fix #50438, use default pool for at-threads -->
- [x] #49031 <!-- Update inference.md -->
- [x] #50289 <!-- Initialize prev_nold and nold in gc_reset_page -->
- [x] #50559 <!-- Expand kwcall lowering positional default check to
vararg -->
- [x] #49582 <!-- Update HISTORY.md for `DelimitedFiles` -->
- [x] #50341 <!-- invokelatest docs should say not exported before 1.9
-->
- [x] #50525 <!-- only check that values are finite in `generic_lufact`
when `check=true` -->
- [x] #50444 <!-- Optimize getfield lowering to avoid boxing in some
cases -->
- [x] #50523 <!-- Avoid generic call in most cases for getproperty -->
- [x] #50860 <!-- Add `Base.get_extension` to docs/API -->
- [x] #50164 <!-- codegen: handle dead code with unsafe_store of FCA
pointers -->
- [x] #50568 <!-- `Array(::AbstractRange)` should return an `Array` -->
- [x] #50871 <!-- macOS: Don't inspect dead threadtls during exception
handling. -->

Need manual backport:
- [ ] #48542 <!-- Add docs on task-specific buffering using
multithreading -->
- [ ] #50591 <!-- build: fix various makefile bugs -->


Non-merged PRs with backport label:
- [ ] #50842 <!-- Avoid race conditions with recursive rm -->
- [ ] #50823 <!-- Make ranges more robust with unsigned indexes. -->
- [ ] #50663 <!-- Fix Expr(:loopinfo) codegen -->
- [ ] #49716 <!-- Update varinfo() docstring signature -->
- [ ] #49713 <!-- prevent REPL from erroring in numbered mode in some
situations -->
- [ ] #49573 <!-- Implement jl_cpu_pause on PPC64 -->
- [ ] #48726 <!-- fix macro expansion of property destructuring -->
- [ ] #48642 <!-- Use gc alloc instead of alloc typed in lowering -->
- [ ] #48183 <!-- Don't use pkgimage for package if any includes fall in
tracked path for coverage or alloc tracking -->
- [ ] #48050 <!-- improve `--heap-size-hint` arg handling -->
- [ ] #47615 <!-- Allow threadsafe access to buffer of type inference
profiling trees -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:linear algebra Linear algebra kind:breaking This change will break code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants