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

union domain implementation #109

Merged
merged 14 commits into from
Jun 14, 2023
Merged

union domain implementation #109

merged 14 commits into from
Jun 14, 2023

Conversation

SpartaKushK
Copy link
Contributor

  • main file
  • docstrings

pina/geometry/union.py Outdated Show resolved Hide resolved
pina/geometry/union.py Outdated Show resolved Hide resolved
pina/geometry/union.py Outdated Show resolved Hide resolved
pina/geometry/union.py Outdated Show resolved Hide resolved
@dario-coscia
Copy link
Collaborator

Please also add tests for the implementation, like in test_cartesian

pina/geometry/union.py Outdated Show resolved Hide resolved
pina/geometry/union.py Outdated Show resolved Hide resolved
tests/test_union.py Show resolved Hide resolved
pina/geometry/union.py Outdated Show resolved Hide resolved
pina/geometry/union.py Outdated Show resolved Hide resolved
pina/geometry/union.py Outdated Show resolved Hide resolved
pina/geometry/union.py Outdated Show resolved Hide resolved
pina/geometry/union.py Outdated Show resolved Hide resolved
tests/test_union.py Outdated Show resolved Hide resolved
@dario-coscia
Copy link
Collaborator

I left some comments, I think that once these are fixed it can be merge from my side.

pina/geometry/union.py Outdated Show resolved Hide resolved
pina/geometry/union.py Outdated Show resolved Hide resolved
pina/geometry/union.py Outdated Show resolved Hide resolved
- main file
- docstrings
- minor edits to union
- added Union to __init__.py of geometry
- CartesianDomains
- EllipsoidDomain
- CartesianDomain and EllipsoidDomain
- renamed tests cases
- add param type for __init__
- added a location check in __init__
- return a label tensor in sample with labels 'x0', 'x1'...
- ensures dimensions of geometries are same
- sampling improved
- test case typo fixed
- make `geometries` private
- formatting
@dario-coscia
Copy link
Collaborator

dario-coscia commented Jun 14, 2023

👋🏻@ndem0 . I have finished the review of @SpartaKushK code. For my side it can be merge, let me know if you think we missed something

@dario-coscia dario-coscia added the enhancement New feature or request label Jun 14, 2023
@ndem0
Copy link
Member

ndem0 commented Jun 14, 2023

yes, well done @SpartaKushK @dario-coscia!

@ndem0 ndem0 merged commit 81f2b4c into mathLab:v0.1 Jun 14, 2023
10 of 12 checks passed
ndem0 pushed a commit that referenced this pull request Jul 27, 2023
* Union Class
* Implemented Union Tests

---------

Co-authored-by: Dario Coscia <[email protected]>
ndem0 pushed a commit that referenced this pull request Nov 17, 2023
* Union Class
* Implemented Union Tests

---------

Co-authored-by: Dario Coscia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants