Skip to content

ulab: actually update the submodule #2811

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

Merged
merged 4 commits into from
Apr 28, 2020
Merged

Conversation

jepler
Copy link

@jepler jepler commented Apr 26, 2020

#2802 missed the submodule update itself.

jepler added 2 commits April 26, 2020 10:12
PR#2802 missed the submodule update itself.
@jepler
Copy link
Author

jepler commented Apr 26, 2020

We may now face the first need to disable a ulab module, such as the newly added comparison module. At least the espruino pico board no longer builds with this PR, due to overflowing flash.

@v923z
Copy link

v923z commented Apr 27, 2020

We may now face the first need to disable a ulab module, such as the newly added comparison module. At least the espruino pico board no longer builds with this PR, due to overflowing flash.

@jepler @tannewt I am willing to change the sub-module structure, if necessity be. I would, perhaps, suggest at this point to run a poll on which functions people need, and use the results as a guideline.

jepler added 2 commits April 27, 2020 07:46
this non-"express" board is nearly full.  Right now it's actually possible
just to disable the "compare" module, but as this leaves it packed
pretty full I prefer to fully disable it in order to avoid the topic
returning again soon.
@jepler
Copy link
Author

jepler commented Apr 27, 2020

I chose to disable ulab altogether on the espruino pico.

@jepler jepler requested a review from tannewt April 27, 2020 15:19
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Looks good to me. Removing on Espruino-pico is a fine choice.

Copy link
Collaborator

@hierophect hierophect left a comment

Choose a reason for hiding this comment

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

Looks good to me. Reminds me to re-check -flto, I can't remember why I didn't enable that last time but it may have been resolved by other recent makefile/linker changes.

@tannewt
Copy link
Member

tannewt commented Apr 27, 2020

Are we able to disable submodules of ulab independently?

@jepler
Copy link
Author

jepler commented Apr 27, 2020

@tannewt yes, we are able to selectively disable parts of ulab. I originally investigated that as a solution, but with the new ulab.comparison module deactivated it left only about 2kB (I forget exactly) of flash free. My gut said, better to fully disable ulab on this board than return within days or weeks to revisit the issue. If we get lto and it reclaims 20+kB as I suspect it would, that would be a great time to re-revisit it. However, I can change this PR to disable as little as possible, if that's what you prefer. Please merge or let me know.

@tannewt
Copy link
Member

tannewt commented Apr 28, 2020

This is fine. Thanks!

@tannewt tannewt merged commit 40a1f3e into adafruit:master Apr 28, 2020
@v923z
Copy link

v923z commented Apr 28, 2020

I chose to disable ulab altogether on the espruino pico.

This brings up another issue, which, perhaps, deserves its own thread: it would be really great, if users could customise their own ulab (or any other external module, for that matter). I have come to realise that in circuitpython the main tripping point is that the makefile, py.mk, has to be modified. Would it be possible to accept arbitrary make fragments from external modules, exactly as is done in micropython? With that, this issue of the espruino pico would be rendered redundant.

I believe, this would benefit everyone, because circuitpython users could then be able to pull directly from the ulab repository, and try whatever they want.

@tannewt
Copy link
Member

tannewt commented Apr 28, 2020

@v923z We should discuss this on a separate issue. Generally, we don't support users integrating their own C code into CircuitPython.

@v923z
Copy link

v923z commented Apr 28, 2020

@v923z We should discuss this on a separate issue.

OK. It is here: #2825

Generally, we don't support users integrating their own C code into CircuitPython.

I understand this, but here seems to be a legitimate reason for changing that.

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.

5 participants