Replacing the concrete material with a better reference

terrapower / armi

An open-source nuclear reactor analysis automation framework that helps design teams increase efficiency and quality

https://terrapower.github.io/armi/

Apache License 2.0

212 stars 82 forks source link

Replacing the concrete material with a better reference #1717

Closed john-science closed 3 weeks ago

john-science commented 1 month ago

What is the change?

Fixing dead link in concrete material

Why is the change being made?

To close #1715 , I am using archive.org so this reference will never go out of data again.

Checklist

[X] This PR has only one purpose or idea.
[X] Tests have been added/updated to verify any new/changed code.

[X] The code style follows good practices.
[X] The commit message(s) follow good practices.

[X] The release notes have been updated if necessary.
[X] The documentation is still up-to-date in the doc folder.
[X] The dependencies are still up-to-date in pyproject.toml.

albeanth commented 1 month ago

I mean... cool. but that PDF is in (presumably) Japanese.... So I have no idea how that's helpful long term lol. And I couldn't find anything concrete related in a quick scan of said document.

I'd rather replace the information with something actually useful. For instance, nist.gov provides the composition of concrete with a density. I think this is a better resource for us to use. https://physics.nist.gov/cgi-bin/Star/compos.pl?matno=144

john-science commented 1 month ago

I'd rather replace the information with something actually useful. For instance, nist.gov provides the composition of concrete...

That would be a science change to ARMI. The NIST values are pretty close, but the numbers don't exactly match.

I suppose no one we know downstream uses this material from ARMI. So it wouldn't be a very important science change.

Hmmm

albeanth commented 1 month ago

That would be a science change to ARMI. The NIST values are pretty close, but the numbers don't exactly match.

I suppose no one we know downstream uses this material from ARMI. So it wouldn't be a very important science change.

Hmmm

Science change, yes, but also a relevant improvement. Relying on data from a potentially questionable source is arguably bad practice -- especially just for the sake of continuity. If someone gripes about it, we can always revert, but I would be pretty surprised if that happened tbh.

I would advocate for continually improving data like this and ensuring that we are providing relevant and (at least somewhat) reliable data for our users.