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.

Sign up for GitHub

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

Jump to bottom

[MPM] Change application name #12011

Merged
merged 56 commits into from
Mar 13, 2024
Merged

[MPM] Change application name #12011

merged 56 commits into from
Mar 13, 2024

Conversation

ncrescenzio
Copy link
Contributor

This PR changes the name of the application implementing the Material Point Method from ParticleMechanicsApplication to MPMApplication

@antonialarese
@KratosMultiphysics/technical-committee

@ncrescenzio
rename files in main folder
cdff96a
@ncrescenzio
updated files using new application name
5b852c8
@ncrescenzio
Renamed files in custom_io
d294b49
@ncrescenzio
Updated files in custom_io
abde946
@ncrescenzio
Renamed file in custom_processes
ac7054a
@ncrescenzio
Updated file in custom_processes
3d2f4c9
@ncrescenzio
Updated files in custom_elements
bc3acd8
@ncrescenzio
Updated files in custom_conditions/grid_based_conditions
1f48ef1
@ncrescenzio
Updated files in `custom_conditions/particle_based_conditions'
a7b6d56
@ncrescenzio
Renamed files in custom_utilities
1887246
@ncrescenzio
Updated files in custom_utilities
7af6680
@ncrescenzio
Renamed file in custom_python
86a4b3d
@ncrescenzio
Updated files in custom_python
3eb84b2
@ncrescenzio
Updated files in custom_strategies
9aba25c
@ncrescenzio
Renamed files in custom_constitutive
23be12e
@ncrescenzio
Updated files in custom_constitutive
0ed0176
@ncrescenzio
Renamed files in python_scripts
8d73628
@ncrescenzio
Updated files in python_scripts
dd993b7
@ncrescenzio
Updated wheels
1fa75af
@ncrescenzio
Updated KratosMultiphysics-all.json
746f6fa
@ncrescenzio
Updated CODEOWNERS
269be38
@ncrescenzio
Updated .github/workflows
92696df
@ncrescenzio
Changee PARTICLES_PER_ELEMENT with MATERIAL_POINTS_PER_ELEMENT an…
7df0d8c 
…d `PARTICLES_PER_CONDITION` with `MATERIAL_POINTS_PER_CONDITION`
@ncrescenzio
fix include files
1f08346
@ncrescenzio
Renamed files in tests
ee54a84
@ncrescenzio
Updated cpp tests
38c17ba
@ncrescenzio
Updated files in tests
da6b1af
@ncrescenzio
Renamed folder ParticleMechanicsApplication in MPMApplication
b3e8b61
@ncrescenzio
tests fixed
cafea57
@ncrescenzio
Renamed solver wrappers in CoSimulationApplication
80238b0
philbucher
philbucher reviewed Feb 5, 2024
View reviewed changes
Copy link
Member

@philbucher philbucher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont have any objections, but I strongly recommend to add backward compatibility
Otherwise you are breaking every single existing case out there
And speaking from experience, others are not happy abt this ;)

applications/CoSimulationApplication/tests/mpm_fem_beam/ProjectParametersMPM.json Outdated
@@ -78,13 +78,13 @@
}]
},
"print_output_process" : [{
"python_module" : "particle_json_output_process",
"kratos_module" : "KratosMultiphysics.ParticleMechanicsApplication",
"python_module" : "mpm_json_output_process",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g. this is not backward compatible

applications/CoSimulationApplication/tests/mpm_fem_beam/cosim_mpm_fem_beam_parameters.json Outdated
{
"type" : "solver_wrappers.kratos.particle_mechanics_dirichlet_wrapper",
"type" : "solver_wrappers.kratos.mpm_dirichlet_wrapper",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@ncrescenzio
Copy link
Contributor Author

@philbucher so should I duplicate all the python files, keeping both the one with the old name and the new one?

@philbucher
Copy link
Member

I would add wrappers with the old name, that create an object of the new type.
There you can issue deprecation warnings so that users can update at their own pace (maybe include a date in the message)

philbucher
philbucher reviewed Mar 3, 2024
View reviewed changes
Copy link
Member

@philbucher philbucher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no objections, looking good from my side.

I am still a bit worried about the backward compatibility, seems like many things have changed

My suggestion is to only update some tests, and leave others with the old input. This way you are sure that the backward compatibility that you implement is working properly

...ionApplication/python_scripts/solver_wrappers/kratos/particle_mechanics_dirichlet_wrapper.py Outdated Show resolved Hide resolved
applications/CoSimulationApplication/tests/mpm_dem/MaterialPointMaterials.json Outdated Show resolved Hide resolved
ncrescenzio added 13 commits March 5, 2024 09:29
@ncrescenzio
Revert "Renamed files in CoSimulationApplication/tests"
bde84aa 
This reverts commit 4d2ed98.
@ncrescenzio
Revert "Updated files in CoSimulationApplication/tests"
15de64e 
This reverts commit dfc726e.
@ncrescenzio
Revert "Updated files in tests"
04aa3f8 
This reverts commit da6b1af.
@ncrescenzio
Added deprecation warning: parameter particles_per_condition replac…
fd081eb 
…ed by `material_points_per_condition`
@ncrescenzio
added wrappers for old python processes
cdccfe7
@ncrescenzio
added wrapper for old analysis stage class
de1cbe1
@ncrescenzio
Added deprecation warning: parameter PARTICLES_PER_ELEMENT replaced…
eaf8b6d 
… by `MATERIAL_POINTS_PER_ELEMENT` in `ParticleMaterials.json`
@ncrescenzio
removed useless import
9667212
@ncrescenzio
updated name test dependencies
2b7bf96
@ncrescenzio
updated application name in tests projectparameters
9f799fc
@ncrescenzio
minor changes to prove backward compatibility
4033126
@ncrescenzio
update python tests
f6019b7
@ncrescenzio
Merge branch 'master' into mpm/change-application-name
855f989
@ncrescenzio
Copy link
Contributor Author

@philbucher as you suggested, I also added the wrappers with the old names of python processes and left the old inputs in the tests.

@philbucher
Copy link
Member

cool, looks good from my side

I leave the approval to an active user of the App, maybe @VeronikaSinger ?

@VeronikaSinger
Copy link
Contributor

Thanks @philbucher for the review and thanks @ncrescenzio for the renaming of the application. I approve

@ncrescenzio
Copy link
Contributor Author

Thanks @philbucher @VeronikaSinger!

@ncrescenzio ncrescenzio merged commit 0f83a7f into master Mar 13, 2024
17 of 21 checks passed
@ncrescenzio ncrescenzio deleted the mpm/change-application-name branch March 13, 2024 09:39
This was referenced Mar 13, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philbucher philbucher philbucher left review comments

@VeronikaSinger VeronikaSinger VeronikaSinger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ncrescenzio @loumalouomega @philbucher @VeronikaSinger