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

Feature/support gradient arrays in firebase #275

Merged
merged 8 commits into from
Aug 12, 2024

Conversation

rugeli
Copy link
Collaborator

@rugeli rugeli commented Jul 16, 2024

Problem

What is the problem this work solves, including
closes #273

Solution

When gradient is a list of gradient names, e.g "gradient": ["X_gradient","Y_gradient"], we need to add additional steps to our current upload+download system.

  • when uploading: added handling for gradient lists to record each reference of gradient in the list
  • for duplicate check: resolve each gradient data in the list correctly and consistently
  • when downloading: prepared data to recognize gradient lists and fetch each gradient data correctly

Note: After adding logic to detect, unpack and resolve combined gradients in the corresponding steps of the process, our system can now upload and download the current test recipes in all contexts. I did notice an odd behavior (not breaking anything) with the recipe loader automatically adding a default representation to each object when downloading recipes from firebase, will write another issue to address this.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Steps to Verify:

  1. Ensure your database doesn't contain any test_combined_gradient.json related data. Delete them if they exist
  2. Upload recipe to firebase: run upload -r cellpack/tests/recipes/v2/test_combined_gradient.json
  3. Pack the firebase recipe: run pack firebase:recipes/test_combined_gradient_v_1.0.0
  4. If followed step 1, you should be able to upload and download without issues. (but please let me know if you do encounter any)

Copy link

github-actions bot commented Jul 16, 2024

Packing analysis report

Analysis for packing results located at cellpack/tests/outputs/test_spheres/spheresSST

Ingredient name Encapsulating radius Average number packed
ext_A 25 236.0

Packing image

Packing image

Distance analysis

Expected minimum distance: 50.00
Actual minimum distance: 50.01

Ingredient key Pairwise distance distribution
ext_A Distance distribution ext_A

@rugeli rugeli requested review from meganrm and mogres July 16, 2024 21:12
@rugeli rugeli marked this pull request as draft July 17, 2024 17:32
Comment on lines +732 to +738
elif key == "gradient" and isinstance(obj_data[key], list):
new_grad_list = []
for grad in obj_data[key]:
for name in grad:
grad_dict[name] = grad[name]
new_grad_list.append(name)
obj_dict[obj_name][key] = new_grad_list
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This section of code shares some similarities with def get_reference_in_obj() in CompositionDoc class, but they perform distinct tasks -- this one unpacks gradients for downloading, the latter resolves db references for duplicate checking while uploading. For clarity and easier maintenance, I'm going to leave them as standalone sections.

@rugeli rugeli marked this pull request as ready for review July 17, 2024 22:27
@rugeli rugeli changed the title Feature/upload gradient arrays Feature/support gradient arrays in firebase Jul 18, 2024
Copy link
Collaborator

@mogres mogres left a comment

Choose a reason for hiding this comment

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

Tested out with the combined gradient recipe and it works for me!

@rugeli rugeli merged commit f95fb36 into main Aug 12, 2024
7 checks passed
@rugeli rugeli deleted the feature/upload-gradient-arrays branch August 12, 2024 19:10
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.

Add support for uploading recipes that include gradient arrays
3 participants