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

Make Ising example configurable #89

Merged
merged 2 commits into from
Jan 19, 2022
Merged

Conversation

streeve
Copy link
Collaborator

@streeve streeve commented Jan 14, 2022

Makes the spin value randomization and non-linear spin function optional. With this update running the function with defaults results in the original Ising model

@streeve streeve requested a review from allaffa January 14, 2022 17:33
@streeve streeve self-assigned this Jan 14, 2022
@@ -100,7 +105,9 @@ def create_dataset(L, histogram_cutoff, dir):
for config in multiset_permutations(primal_configuration):
config = np.array(config)
config = np.reshape(config, (L, L, L))
total_energy, atomic_features = E_dimensionless(config, L)
total_energy, atomic_features = E_dimensionless(
config, L, spin_function
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
config, L, spin_function
config, L, spin_function, scale_spin

Copy link
Collaborator

Choose a reason for hiding this comment

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

Did we have a unit test for Ising model? Not sure why the checks did not catch this

Copy link
Collaborator

@allaffa allaffa Jan 16, 2022

Choose a reason for hiding this comment

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

there are currently no unit tests that use this model.

spin = np.zeros_like(config)

if scale_spin:
random_scaling = np.random.random((L ** 3,))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
random_scaling = np.random.random((L ** 3,))
random_scaling = np.random.random((L, L, L))

Copy link
Collaborator

Choose a reason for hiding this comment

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

@pzhanggit
if you propose this change of dimensionality, just make sure that this change of dimensionality does not break the np.multiply in the following line that perform element-wise multiplication between lumpy arrays

Copy link
Collaborator

@pzhanggit pzhanggit Jan 16, 2022

Choose a reason for hiding this comment

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

@allaffa That's exactly why I proposed the change. The original code failed with dimension inconsistency error. I changed it to fix the bugs. Same as line 109. I can unfix them if you want

for z in range(L):
for y in range(L):
for x in range(L):
spin[x, y, z] = math.sin(math.pi * config[x, y, z] / 2)
spin[x, y, z] = spin_function(config[x, y, z])
Copy link
Collaborator

Choose a reason for hiding this comment

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

@streeve
smart :D

@pzhanggit pzhanggit merged commit 2dbc0c6 into ORNL:main Jan 19, 2022
RylieWeaver pushed a commit to RylieWeaver/HydraGNN that referenced this pull request Oct 17, 2024
Make Ising example configurable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants