maml icon indicating copy to clipboard operation
maml copied to clipboard

Question about radius in BOWSR readme example

Open janosh opened this issue 1 year ago • 7 comments

This might be a dumb question. In the BOWSR readme example, why the rounding? And is the 0.6 factor an approximation for getting the larger element's radius?

expected_radius() might be better named expected_diameter() since it gives the sum of 2 radii.

https://github.com/materialsvirtuallab/maml/blob/a37f78a090c338a79f4c0644e5e267c60c50a8d1/maml/apps/bowsr/README.md?plain=1#L75-L81

janosh avatar Sep 21 '22 20:09 janosh

@JiQi535 Pls fix and respond.

shyuep avatar Sep 21 '22 20:09 shyuep

@JiQi535 In case you rename the function, I think it can also be simplified from this:

def expected_radius(struct):
    element_list = struct.composition.chemical_system.split("-")
    element_list = [get_el_sp(e) for e in element_list]
    ele1, ele2 = sorted(element_list, key=lambda x: x.atomic_radius)[:2]
    return ele1.atomic_radius + ele2.atomic_radius

to this:

def expected_diameter(structure: Structure) -> float:
    elem_1, elem_2, *_ = sorted(structure.composition.elements, key=lambda x: x.atomic_radius)
    return elem_1.atomic_radius + elem_2.atomic_radius

janosh avatar Sep 21 '22 23:09 janosh

Btw, this function fails for unary structures.

ValueError: not enough values to unpack (expected at least 2, got 1)

janosh avatar Sep 22 '22 03:09 janosh