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

[Merged by Bors] - feat(ring_theory/discrete_valuation_ring): add additive valuation #5094

Closed
wants to merge 10 commits into from

Conversation

kbuzzard
Copy link
Member

@kbuzzard kbuzzard commented Nov 23, 2020

Following the approach used for p-adic numbers, we define an additive valuation on a DVR R as a bare function v: R -> nat, with the convention that v(0)=0 instead of +infinity for convenience. Note that we have no hom structure (like monoid_hom or add_monoid_hom) for v(a*b)=v(a)+v(b) and anyway this doesn't even hold if ab=0. We prove all the usual axioms.


The "correct" thing to define is a multiplicative map but the ideal target would be with_zero (multiplicative nat), and we can't use valuations for this right now, because this isn't a group with zero. In future I propose to change the target of a valuation from a group_with_zero to a monoid_with_zero (still totally ordered), but I have to make two more ordered monoid classes before I can do this the way I want to do it.

@kbuzzard kbuzzard added the awaiting-review The author would like community review of the PR label Nov 23, 2020
-/

/-- The `ℕ`-valued additive valuation on a DVR (returns junk at `0` rather than `+∞`) -/
noncomputable def add_val (R : Type u) [integral_domain R] [discrete_valuation_ring R] : R → ℕ :=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's not much point, but you could declare this a zero_hom R ℕ to get a tiny bit of bundling

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I almost sent 0 to 37. This definition is really an auxiliary one; I will only be using it when defining the correct object, which is either the map to with_top nat sending 0 to top, or (more likely) the map to with_zero (multiplicative nat) or with_zero (multiplicative int), because the latter is the valuation right now (and the former isn't, but will be once I get round to generalising the definition of valuation).

Should I rename this function add_val_aux and define a new function add_val to with_top nat? This would have more functorial properties.

Copy link
Member

@jcommelin jcommelin Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding _aux to the name is a useful flag that this is meant as implementation detail. You could also explicitly state that in the docstring.
Otoh, if we think we'll be using this later on, then I'd prefer to keep the name the way it is.

Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took me way to long to take a look at this. LGTM.

Copy link
Collaborator

@Vierkantor Vierkantor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM otherwise.

λ r, if hr : r = 0 then 0 else
classical.some (associated_pow_irreducible hr (classical.some_spec $ exists_irreducible R))

theorem add_val_spec {r : R} (hr : r ≠ 0) :
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add_val_def is the more friendly version intended for general use, right?

Suggested change
theorem add_val_spec {r : R} (hr : r ≠ 0) :
/-- `add_val_spec` is a specific instantiation of `add_val_def` used to prove the latter. -/
theorem add_val_spec {r : R} (hr : r ≠ 0) :

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, add_val_def is the useful version. Even though it's not the actual definition, it's the "mathematician's definition" of the additive valuation. I'm not sure if add_val_spec is a specific instantiation of add_val_def -- I'm not really sure I understand what that means.

src/ring_theory/discrete_valuation_ring.lean Outdated Show resolved Hide resolved
Copy link
Member

@robertylewis robertylewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, with the minor suggestions!

bors d+

src/ring_theory/discrete_valuation_ring.lean Outdated Show resolved Hide resolved
src/ring_theory/discrete_valuation_ring.lean Outdated Show resolved Hide resolved
@bors
Copy link

bors bot commented Dec 2, 2020

✌️ kbuzzard can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@robertylewis robertylewis added awaiting-author A reviewer has asked the author a question or requested changes delegated The PR author may merge after reviewing final suggestions. and removed awaiting-review The author would like community review of the PR labels Dec 2, 2020
kbuzzard and others added 4 commits December 4, 2020 00:21
Co-authored-by: Rob Lewis <Rob.y.lewis@gmail.com>
Co-authored-by: Anne Baanen <Vierkantor@users.noreply.github.com>
Co-authored-by: Rob Lewis <Rob.y.lewis@gmail.com>
@robertylewis
Copy link
Member

bors merge

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-author A reviewer has asked the author a question or requested changes labels Dec 4, 2020
bors bot pushed a commit that referenced this pull request Dec 4, 2020
)

Following the approach used for p-adic numbers, we define an additive valuation on a DVR R as a bare function v: R -> nat, with the convention that v(0)=0 instead of +infinity for convenience. Note that we have no `hom` structure (like `monoid_hom` or `add_monoid_hom`) for v(a*b)=v(a)+v(b) and anyway this doesn't even hold if ab=0. We prove all the usual axioms.
@bors
Copy link

bors bot commented Dec 4, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(ring_theory/discrete_valuation_ring): add additive valuation [Merged by Bors] - feat(ring_theory/discrete_valuation_ring): add additive valuation Dec 4, 2020
@bors bors bot closed this Dec 4, 2020
@bors bors bot deleted the DVR-add-valuation branch December 4, 2020 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
delegated The PR author may merge after reviewing final suggestions. ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants