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

Replacing the concrete material with a better reference #1717

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

john-science
Copy link
Member

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

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

@john-science john-science added the documentation Improvements or additions to documentation label May 29, 2024
@john-science john-science requested a review from albeanth May 29, 2024 17:49
@albeanth
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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.

@john-science john-science changed the title Fixing dead link in concrete material Replacing the concrete material with a better reference May 31, 2024
@john-science john-science merged commit f06e2bf into main Jun 3, 2024
23 checks passed
@john-science john-science deleted the concrete_reference branch June 3, 2024 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dead link for concrete.py
2 participants