search

prisms-center / phaseField

PRISMS-PF: An Open-Source Phase-Field Modeling Framework
https://prisms-center.github.io/phaseField/
Other
229 stars 120 forks source link

input parameter inconsistency for weightedDistanceFromNucleusCenter function #200

Closed zzzmx-josh closed 1 month ago

zzzmx-josh commented 11 months ago

In the seedNucleus function located in applications/_nucleating_precipitates_MgRE/equations.h, there seems to be a discrepancy related to the usage of the weightedDistanceFromNucleusCenter method. The function requires the nucleus_rot_matrix, calculated by get_nucleus_rotation_matrix, as a parameter. However, the original weightedDistanceFromNucleusCenter method does not require this rotation matrix as a parameter. Instead, the rotation matrix is directly used in the method through shortest_edist_tensor = userInputs.get_nucleus_rotation_matrix(var_index) * shortest_edist_tensor.

As a result, compiling this app as it stands might result in an error due to this inconsistency. Should I consider removing the nucleus_rot_matrix input parameter from the seedNucleus function to align it with the weightedDistanceFromNucleusCenter method's logic ？

david-montiel-t commented 11 months ago

Thank you for reporting this issue, Josh. I will have a look.

On Tue, Dec 12, 2023, 8:58 AM zzzmx_josh @.***> wrote:

In the seedNucleus function located in applications/_nucleating_precipitates_MgRE/equations.h, there seems to be a discrepancy related to the usage of the weightedDistanceFromNucleusCenter method. The function requires the nucleus_rot_matrix, calculated by get_nucleus_rotation_matrix, as a parameter. However, the original weightedDistanceFromNucleusCenter method does not require this rotation matrix as a parameter. Instead, the rotation matrix is directly used in the method through shortest_edist_tensor = userInputs.get_nucleus_rotation_matrix(var_index) * shortest_edist_tensor.

As a result, compiling this app as it stands might result in an error due to this inconsistency. Should I consider removing the nucleus_rot_matrix input parameter from the seedNucleus function to align it with the weightedDistanceFromNucleusCenter method's logic ？

— Reply to this email directly, view it on GitHub https://github.com/prisms-center/phaseField/issues/200, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD3FA46PYJTNKO5XOY6MKITYJBPITAVCNFSM6AAAAABARQS4MCVHI2DSMVQWIX3LMV43ASLTON2WKOZSGAZTOOBRGU4DINQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

landinjm commented 1 month ago

I don't believe an overload for this function ever existed with a rotation matrix input. Given that _nucleating_precipitates_MgRE was a work in progress application (that has now been deprecated), I am marking this as won't fix.

landinjm commented 1 month ago

Marking as wontfix