Implementing new API for NDK #122
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NewtonSander
force-pushed
the
implementing_procedural_api
branch
2 times, most recently
from
55f9830
to
5ec2529
Compare
NewtonSander
force-pushed
the
implementing_procedural_api
branch
from
b2e7bce
to
de893ab
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #122 +/- ##
==========================================
+ Coverage 73.15% 73.81% +0.66%
==========================================
Files 27 30 +3
Lines 2097 2074 -23
==========================================
- Hits 1534 1531 -3
+ Misses 563 543 -20
☔ View full report in Codecov by Sentry. |
NewtonSander
force-pushed
the
implementing_procedural_api
branch
from
1a5d57d
to
0f80407
Compare
…frequency is changed
NewtonSander
force-pushed
the
implementing_procedural_api
branch
from
b8fea20
to
a9e4f94
Compare
…aterial_layers, using material_masks keys instead
…sition, origin and direction
NewtonSander
force-pushed
the
implementing_procedural_api
branch
from
fe3f6f9
to
22b9b30
Compare
|
@charlesincharge I've addressed all your suggestions and also made some other changes, could you have another look? |
charlesbmi
approved these changes
Aug 7, 2023
1 task
NewtonSander
added a commit
that referenced
this pull request
Jul 26, 2024
* Implementing new API for NDK * Updating gallery examples to use the new API * Updating 3d scenario result * Cleaning up scenario __init__ * Updating api docs * Compiling problem explicitly * Small fixes to plot full scenario * Creating NDK's own Problem class * Removing 'scenario_id' attribute * Cleaning up * Fixing docs * Decoupling render layout from compile problem * Moving grid creation to its own class * Moving center_frequency to the scenario root * Cleaning up * Cleaning up code. Invalidating grid and compiled problem when center_frequency is changed * Adapting documentation * Updating plot full scenario to use the procedural approach * Updating API docs adding the Grid and Problem pages * Fixing bug with 3D example * Improving docs, updating hash of 3d results file * Improving docs * Small improvements to notebook * Addressing review suggestion: replacing enum with subdirectory containing the built in scenarios * Update src/neurotechdevkit/grid.py Co-authored-by: charles <[email protected]> * Addressing review suggestion * Fixing issue with commited PR suggestion * Fixing bug * Removing layer_ids from problem creation. Removing need of creating material_layers, using material_masks keys instead * Fixing print * Replacing numpy arrays with plain lists for simple attributes like position, origin and direction * Refactoring * Refactoring * Fixing hash of results file * Simplifying API * Setting center_frequency in built_in scenarios * Adding SliceAxis to spellcheck's whitelist * Adding unit tests and cleaning up code * Adding predefined target options * Fix typo * Showing how the predefined targt options can be used * Improving plot full scenario masks creation * Addressing review suggestions * Fixing spellchecking error by removing unnecessary types from docstrings * Updating 3d file hash * update docstrings --------- Co-authored-by: charles <[email protected]> Co-authored-by: d-lucena <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduction
This PR brings a suggested new API for NDK, the main goals are:
Scenarioand implement abstract methods when creating a new scenario.The scenarios NDK already supported were slightly changed in order to conform to the new API
An example of how the procedural API can be used is the Full Scenario Example