-
Notifications
You must be signed in to change notification settings - Fork 6
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
Working on Bingham Material #13
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use black
to format the code with line length 88
I had a doubt in bingham.tcc file, in the function compute stress can you please tell what is the state_vars. |
I haven't implemented state variables yet but I think it's just a dictionary. See |
Yes, it's a dict to store variables specific to individual material points |
In the particles.py class the init function does not have dvolumetric_strain while it has been computed in later functions of the same class. |
Thanks for pointing that out. I added it to the init method. The reason it worked fine was that every time a new value is assigned to it so it was not needed to be carried over by jax (un/)flattening but better to have it there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add a couple more things?
- Add a test case
- Add docstrings & comments wherever needed to document its working
- Check dimensions of the variables. (I think I saw some operations that might raise an error)
I've also added a few more comments. Let me know if you have questions or need any help!
In the test cases of bingham material I am getting a deviation from the answer present in the CB-Geo test case from bingham in case 3 and 4 which are for yielded case, The tau value is coming off by a value of 20-30 I have checked the code multiple times but am unable to find any error, the dvolumetric strain is coming correct. I think the strain rate has some error it is coming [-0.25,-0.375,0.0,-0.625,0,0] can you please see once |
@SachinJalan Let me see if I can find something. Can you be a little clearer about where you think the error is stemming from? Trying to pin in the issue - you say |
diffmpm/material.py
Outdated
Arguments | ||
--------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arguments | |
--------- | |
Parameters | |
---------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're close to wrapping this up. Just a few small comments.
tests/test_bingham.py
Outdated
{"pressure": jnp.zeros((2, 1))}), | ||
], | ||
) | ||
def test_compute_stress_two_particles(particles, state_vars, element, target, dt): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above. Just declare dt
in the test directly.
tests/test_bingham.py
Outdated
@pytest.fixture | ||
def state_vars(): | ||
return {"pressure": jnp.zeros(1)} | ||
|
||
|
||
@pytest.fixture | ||
def dt(): | ||
return 1.0 | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"particles, element, target", | ||
particles_element_targets, | ||
) | ||
def test_compute_stress(particles, state_vars, element, target, dt): | ||
particles.update_natural_coords(element) | ||
if element.constraints: | ||
element.apply_boundary_constraints() | ||
particles.compute_strain(element, dt) | ||
stress = particles.material.compute_stress(None, particles, state_vars) | ||
assert jnp.allclose(stress, target) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pytest.fixture | |
def state_vars(): | |
return {"pressure": jnp.zeros(1)} | |
@pytest.fixture | |
def dt(): | |
return 1.0 | |
@pytest.mark.parametrize( | |
"particles, element, target", | |
particles_element_targets, | |
) | |
def test_compute_stress(particles, state_vars, element, target, dt): | |
particles.update_natural_coords(element) | |
if element.constraints: | |
element.apply_boundary_constraints() | |
particles.compute_strain(element, dt) | |
stress = particles.material.compute_stress(None, particles, state_vars) | |
assert jnp.allclose(stress, target) | |
@pytest.mark.parametrize( | |
"particles, element, target", | |
particles_element_targets, | |
) | |
def test_compute_stress(particles, element, target): | |
state_vars = {"pressure": jnp.zeros(1)} | |
dt = 1 | |
particles.update_natural_coords(element) | |
if element.constraints: | |
element.apply_boundary_constraints() | |
particles.compute_strain(element, dt) | |
stress = particles.material.compute_stress(None, particles, state_vars) | |
assert jnp.allclose(stress, target) |
No need to mark them as fixtures if they're just returning a value for one test. Just use them in the test directly.
diffmpm/material.py
Outdated
# Compute the stress | ||
def compute_stress(self, dstrain, particles, state_vars:dict): | ||
""" | ||
Computes the stress for the Bingham material |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Computes the stress for the Bingham material | |
Computes the stress for the Bingham material. |
Issue #9