JASory / Iridium

Atomic Physics Library
https://rust-cas.org
MIT License
18 stars 5 forks source link

WIP: NuclideFraction and Element #9

Closed jan-grimo closed 7 months ago

jan-grimo commented 7 months ago

Closes #8.

Initial impl of NuclideFraction and Element

Impl cleanups

I was hoping you could give me some feedback on the direction this is taking @JASory. So far, I think NuclideFraction has limited use since all except one of its methods implementing ChemElement really just forward to one of its Nuclides, since it is (so far) limited to single element type fractions.

Either NuclideFraction stays as-is, with limited usefulness really aside from the fact that library clients can look up the exact isotopic composition of natural samples, or we could drop the restriction, not implement ChemElement, and instead look at implementing some isotope decay features with it.

What do you think?

Aside from that, I know I've meddled a fair bit in your Nuclide impls. I can happily remove those changes from the MR, but I thought they added clarity or also possibly performance in some cases. Just let me know how you feel about them.

TODO

JASory commented 7 months ago

It looks good, thank-you for cleaning up the code the majority was written back when I was writing "C but in Rust".

It seems like both of our initial goals were satisfied, adding generic Element structs while still being user-definable fractions for versatility.

I was planning on extending it to provide for some decay properties, like the approximate decay energy per atom. If you have other ideas let me know.

"Is so far limited to single element fractions"

Unfortunately I intend for it to remain so. Nuclide is meant to model individual nuclides, theoretical nuclides or a nuclide representing an average of some sort, going to model mixtures of different elements is too far. I don't want it to feature-creep into a computational chemistry library, but stick to the same direction. That being said, if there is functionality that makes sense for nuclides and helps with chemistry or other applications I would be willing to implement it.

JASory commented 7 months ago

I'm not sure the purpose of printing the nucleon_fractions argument in from_nucleon_fractions, since the user can just print it before it's passed to the function if they want to. So I'm removing it.

jan-grimo commented 7 months ago

Unfortunately I intend for it to remain so.

A-okay with me. I just felt it had limited utility and wanted to offer some options.

So I'm removing it.

Of course. I titled this Work In Progress on purpose, I had actually meant on cleaning this up a bit more, removing the dbg statements and adding documentation :see_no_evil: